On 04/15/2012 09:22 PM, Osier Yang wrote: > On 2012幎04æ??13æ?¥ 22:09, Cole Robinson wrote: >> On 04/13/2012 09:03 AM, Osier Yang wrote: >>> On 04/13/2012 07:50 PM, Cole Robinson wrote: >>>> lvcreate want's the parent pool's name, not the pool path >>>> lvchange and lvremove want lv specified as $vgname/$lvname >>>> >>>> This largely worked before because these commands strip off a >>>> starting /dev. But https://bugzilla.redhat.com/show_bug.cgi?id=714986 >>>> is from a user using a 'nested VG' that was having problems. >>>> >>>> I couldn't find any info on nested LVM and the reporter never responded, >>>> but I reproduced with XML that specified a valid source name, and >>>> set target path to a symlink. >>>> >>>> Signed-off-by: Cole Robinson<crobinso@xxxxxxxxxx> >>>> --- >>>> src/storage/storage_backend_logical.c | 21 +++++++++++---------- >>>> 1 files changed, 11 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/src/storage/storage_backend_logical.c >>>> b/src/storage/storage_backend_logical.c >>>> index 6a235f6..9a91dd9 100644 >>>> --- a/src/storage/storage_backend_logical.c >>>> +++ b/src/storage/storage_backend_logical.c >>>> @@ -672,7 +672,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, >>>> char size[100]; >>>> const char *cmdargvnew[] = { >>>> LVCREATE, "--name", vol->name, "-L", size, >>>> - pool->def->target.path, NULL >>>> + pool->def->source.name, NULL >>> >>> This makes sense. >>> >>>> }; >>>> const char *cmdargvsnap[] = { >>>> LVCREATE, "--name", vol->name, "-L", size, >>>> @@ -778,23 +778,23 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn >>>> ATTRIBUTE_UNUSED, >>>> unsigned int flags) >>>> { >>>> int ret = -1; >>>> + char *volpath = NULL; >>>> >>>> virCommandPtr lvchange_cmd = NULL; >>>> virCommandPtr lvremove_cmd = NULL; >>>> >>>> virCheckFlags(0, -1); >>>> >>>> - virFileWaitForDevices(); >>>> + if (virAsprintf(&volpath, "%s/%s", >>>> + pool->def->source.name, vol->name)< 0) { >>>> + virReportOOMError(); >>>> + goto cleanup; >>>> + } >>>> >>>> - lvchange_cmd = virCommandNewArgList(LVCHANGE, >>>> - "-aln", >>>> - vol->target.path, >>>> - NULL); >>>> + virFileWaitForDevices(); >>>> >>>> - lvremove_cmd = virCommandNewArgList(LVREMOVE, >>>> - "-f", >>>> - vol->target.path, >>>> - NULL); >>>> + lvchange_cmd = virCommandNewArgList(LVCHANGE, "-aln", volpath, NULL); >>>> + lvremove_cmd = virCommandNewArgList(LVREMOVE, "-f", volpath, NULL); >>> >>> I tried with both vol->target.path, and $vgname/$lvname, both >>> of them work. So do we really need these changes? >> >> They both work, just like lvcreate /dev/myvgname also works. But we do need >> this if this 'nested lvm' thing actually exists as the reporter mentioned, or >> user uses a symlink as the target path (I'm not saying that's a valid use case >> but it's a data point). >> >> Also the man pages for lvremove and lvchange but use that format in their >> examples. >> >> Thanks, >> Cole > > Makes sense, and ACK then. > Thanks, pushed now. - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list