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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]