[libvirt] [PATCH 4/8] storage: lvm: Separate creating of the volume from building

Michael Chapman mike at very.puzzling.org
Wed Feb 12 06:29:30 UTC 2014


On Thu, 16 Jan 2014, Peter Krempa wrote:
> On 01/09/14 23:40, Eric Blake wrote:
>> On 01/06/2014 09:44 AM, Peter Krempa wrote:
>>> Separate the steps to create libvirt's volume metadata from the actual
>>> volume building process. This is already done for regular file based
>>> pools to allow job support for storage APIs.
>>> ---
>>>  src/storage/storage_backend_logical.c | 60 +++++++++++++++++++++--------------
>>>  1 file changed, 37 insertions(+), 23 deletions(-)
>>>
>>
>> ACK; I'm borderline whether this should wait for the release, though.
>>
>
> Now that 1.2.2 is out I've pushed this one and the rest with the same
> complaint.

Hi Peter,

This breaks volume creation in an LVM pool:

   # cat volume.xml
   <volume type='block'>
     <name>test</name>
     <capacity unit='bytes'>10737418240</capacity>
     <allocation unit='bytes'>10737418240</allocation>
   </volume>
   # virsh vol-create vg volume.xml
   error: Failed to create vol from volume.xml
   error: key in virGetStorageVol must not be NULL

The problem is a storage backend's createVol function is expected to set 
an appropriate key, but for an LVM volume this isn't done now until 
buildVol is called. The LV's UUID is used as the volume's key.

vol-clone and vol-create-from are even worse: libvirtd crashes since 
the NULL return from virGetStorageVol isn't handled in 
storageVolCreateXMLFrom. We probably need:

   --- a/src/storage/storage_driver.c
   +++ b/src/storage/storage_driver.c
   @@ -1828,6 +1828,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
        pool->volumes.objs[pool->volumes.count++] = newvol;
        volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name,
                                  newvol->key, NULL, NULL);
   +    if (!volobj) {
   +        pool->volumes.count--;
   +        goto cleanup;
   +    }

        /* Drop the pool lock during volume allocation */
        pool->asyncjobs++;

Even with this fix, the LVM backend's buildVolFrom function doesn't look 
like it will ever actually create the LV.

- Michael




More information about the libvir-list mailing list