Re: [libvirt] [PATCH 4/8] storage: lvm: Separate creating of the volume from building
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