[libvirt] fix change-media bug on disk block type and support volume type
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053 When cdrom is block type, the virsh change-media failed to insert source info because virsh uses "<source block='/dev/sdb'/>" while the correct name of the attribute for block disks is "dev". Correct XML: <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> <source dev='/dev/sdb'/> <target dev='vdb' bus='virtio'/> <readonly/> </disk> And, this patch supports cdrom with volume type for change-media command For example: '/var/lib/libvirt/images/boot.iso' is a volume path of 'boot.iso' volume on 'default' pool and the cdrom device has no source. Virsh command: virsh change-media rhel6qcow2 vdb /var/lib/libvirt/images/boot.iso --insert The updated disk XML: <disk type='volume' device='cdrom'> <driver name='qemu' type='raw'/> <source pool='default' volume='boot.iso'/> <target dev='vdb' bus='virtio'/> <readonly/> </disk> Guannan Ren(3) [PATCH 1/3] qemu: throw original error when failing to lookup pool or volume [PATCH 2/3] qemu: support updating pool and volume info when disk is volume type [PATCH 3/3] virsh: fix change-media bug on disk block type and support volume type src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 11 ++++++++++- src/qemu/qemu_driver.c | 5 +++++ tools/virsh-domain.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++------ 6 files changed, 69 insertions(+), 8 deletions(-)
--- src/qemu/qemu_conf.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 094f9f7..d616d7a 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1466,6 +1466,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, virStoragePoolPtr pool = NULL; virStorageVolPtr vol = NULL; virStorageVolInfo info; + virErrorPtr orig_err = NULL; int ret = -1; if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) @@ -1475,7 +1476,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, return 0; if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool))) - return -1; + goto cleanup; if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume))) goto cleanup; @@ -1506,7 +1507,15 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, def->srcpool->voltype = info.type; ret = 0; cleanup: + if (ret < 0 && !orig_err) + orig_err = virSaveLastError(); + virStoragePoolFree(pool); virStorageVolFree(vol); + + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } return ret; } -- 1.8.1.4
--- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 5 +++++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..776c1ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1152,7 +1152,7 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) VIR_FREE(def); } -static void +void virDomainDiskSourcePoolDefFree(virDomainDiskSourcePoolDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a71d6c..ce8e744 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2125,6 +2125,7 @@ bool virDomainObjTaint(virDomainObjPtr obj, void virDomainResourceDefFree(virDomainResourceDefPtr resource); void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); +void virDomainDiskSourcePoolDefFree(virDomainDiskSourcePoolDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9d5f74b..1e7e7e2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -161,6 +161,7 @@ virDomainDiskProtocolTransportTypeToString; virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSourcePoolDefFree; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainEmulatorPinAdd; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a76f14..7ee1c47 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6322,6 +6322,11 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, } if (disk->format) orig->format = disk->format; + if (disk->srcpool) { + virDomainDiskSourcePoolDefFree(orig->srcpool); + orig->srcpool = disk->srcpool; + disk->srcpool = NULL; + } disk->src = NULL; break; -- 1.8.1.4
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053 When cdrom is block type, the virsh change-media failed to insert source info because virsh uses "<source block='/dev/sdb'/>" while the correct name of the attribute for block disks is "dev". Correct XML: <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> <source dev='/dev/sdb'/> <target dev='vdb' bus='virtio'/> <readonly/> </disk> And, this patch supports cdrom with volume type for change-media command For example: '/var/lib/libvirt/images/boot.iso' is a volume path of 'boot.iso' volume on 'default' pool Virsh command: virsh change-media rhel6qcow2 vdb /var/lib/libvirt/images/boot.iso The updated disk XML: <disk type='volume' device='cdrom'> <driver name='qemu' type='raw'/> <source pool='default' volume='boot.iso'/> <target dev='vdb' bus='virtio'/> <readonly/> </disk> --- tools/virsh-domain.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0402aef..e096270 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9594,13 +9594,58 @@ typedef enum { VSH_PREPARE_DISK_XML_UPDATE, } vshPrepareDiskXMLType; +static xmlNodePtr +vshPrepareDiskXMLNode(virConnectPtr conn, + const char *disk_type, + const char *source) +{ + xmlNodePtr node; + node = xmlNewNode(NULL, BAD_CAST "source"); + + if (STREQ(disk_type, "volume")) { + virStoragePoolPtr pool; + virStorageVolPtr vol; + vol = virStorageVolLookupByPath(conn, source); + + if (vol == NULL) { + vshError(NULL, _("failed to get vol '%s'"), source); + goto error; + } + + pool = virStoragePoolLookupByVolume(vol); + if (pool == NULL) { + vshError(NULL, "%s", _("failed to get parent pool")); + virStorageVolFree(vol); + goto error; + } + + xmlNewProp(node, BAD_CAST "pool", BAD_CAST virStoragePoolGetName(pool)); + xmlNewProp(node, BAD_CAST "volume", BAD_CAST virStorageVolGetName(vol)); + + virStoragePoolFree(pool); + virStorageVolFree(vol); + + } else if (STREQ(disk_type, "block")) { + xmlNewProp(node, BAD_CAST "dev",BAD_CAST source); + } else { + xmlNewProp(node, BAD_CAST disk_type, BAD_CAST source); + } + + return node; + +error: + xmlFreeNode(node); + return NULL; +} + /* Helper function to prepare disk XML. Could be used for disk * detaching, media changing(ejecting, inserting, updating) * for changeable disk. Returns the processed XML as string on * success, or NULL on failure. Caller must free the result. */ static char * -vshPrepareDiskXML(xmlNodePtr disk_node, +vshPrepareDiskXML(vshControl *ctl, + xmlNodePtr disk_node, const char *source, const char *path, int type) @@ -9646,9 +9691,9 @@ vshPrepareDiskXML(xmlNodePtr disk_node, } if (source) { - new_node = xmlNewNode(NULL, BAD_CAST "source"); - xmlNewProp(new_node, (const xmlChar *)disk_type, - (const xmlChar *)source); + new_node = vshPrepareDiskXMLNode(ctl->conn, disk_type, source); + if (new_node == NULL) + goto error; xmlAddChild(disk_node, new_node); } else if (type == VSH_PREPARE_DISK_XML_INSERT) { vshError(NULL, _("No source is specified for inserting media")); @@ -9793,7 +9838,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (!(disk_node = vshFindDisk(doc, target, VSH_FIND_DISK_NORMAL))) goto cleanup; - if (!(disk_xml = vshPrepareDiskXML(disk_node, NULL, NULL, + if (!(disk_xml = vshPrepareDiskXML(ctl, disk_node, NULL, NULL, VSH_PREPARE_DISK_XML_NONE))) goto cleanup; @@ -10012,7 +10057,7 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd) if (!(disk_node = vshFindDisk(doc, path, VSH_FIND_DISK_CHANGEABLE))) goto cleanup; - if (!(disk_xml = vshPrepareDiskXML(disk_node, source, path, prepare_type))) + if (!(disk_xml = vshPrepareDiskXML(ctl, disk_node, source, path, prepare_type))) goto cleanup; if (virDomainUpdateDeviceFlags(dom, disk_xml, flags) != 0) { -- 1.8.1.4
participants (1)
-
Guannan Ren