[libvirt] [PATCH] check driver name while attaching disk

This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully # virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save # virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' ============= We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL && + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR, -- 1.7.1

On Mon, Mar 07, 2011 at 02:35:49PM +0800, Wen Congyang wrote:
This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' =============
We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL && + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR,
ACK, though the check for 'DriverName != NULL' is redundant, since the XML parser guarentees this is non-NULL these days. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

At 03/07/2011 07:17 PM, Daniel P. Berrange Write:
On Mon, Mar 07, 2011 at 02:35:49PM +0800, Wen Congyang wrote:
This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' =============
We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL && + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR,
ACK, though the check for 'DriverName != NULL' is redundant, since the XML parser guarentees this is non-NULL these days.
The driverName can be null when we set allow_disk_format_probing to 1 and we do not specify a driverName in xml.
Daniel

At 03/07/2011 07:17 PM, Daniel P. Berrange Write:
On Mon, Mar 07, 2011 at 02:35:49PM +0800, Wen Congyang wrote:
This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' =============
We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL && + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR,
ACK, though the check for 'DriverName != NULL' is redundant, since the XML parser guarentees this is non-NULL these days.
Hi, Daniel PB, and Eric Blake Can you push this patch?
Daniel

On 03/17/2011 10:36 PM, Wen Congyang wrote:
At 03/07/2011 07:17 PM, Daniel P. Berrange Write:
On Mon, Mar 07, 2011 at 02:35:49PM +0800, Wen Congyang wrote:
This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' =============
We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk.
Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL&& + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name,&cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR, ACK, though the check for 'DriverName != NULL' is redundant, since the XML parser guarentees this is non-NULL these days. Hi, Daniel PB, and Eric Blake Can you push this patch?
I just pushed this patch (after verifying that Wen is indeed correct - it is possible for driverName to be NULL if driver->allowDiskFormatProbing is true when qemyCreateCapabilities is called, and later no name is given in the XML being parsed.)

At 03/18/2011 12:40 PM, Laine Stump Write:
On 03/17/2011 10:36 PM, Wen Congyang wrote:
At 03/07/2011 07:17 PM, Daniel P. Berrange Write:
On Mon, Mar 07, 2011 at 02:35:49PM +0800, Wen Congyang wrote:
This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' =============
We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk.
Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL&& + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name,&cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR, ACK, though the check for 'DriverName != NULL' is redundant, since the XML parser guarentees this is non-NULL these days. Hi, Daniel PB, and Eric Blake Can you push this patch?
I just pushed this patch (after verifying that Wen is indeed correct - it is possible for driverName to be NULL if driver->allowDiskFormatProbing is true when qemyCreateCapabilities is called, and later no name is given in the XML being parsed.)
Thanks.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Thanks Wen. However, this patch only adds the checking but to make it work we really have to replace the driver name 'file' with 'qemu', don't we? Thank you very much. Shi On Sun, Mar 6, 2011 at 11:35 PM, Wen Congyang <wency@cn.fujitsu.com> wrote:
This bug was reported by Shi Jin(jinzishuai@gmail.com): ============= # virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb --driver file --subdriver qcow2 Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save error: Failed to restore domain from /var/lib/libvirt/images/memory.save error: internal error unsupported driver name 'file' for disk '/var/lib/libvirt/images/test3.img' =============
We have checked the driver name when we start or restore VM, but we do not check it while attaching a disk.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2892dfe..e94080d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3994,6 +3994,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->driverName != NULL && + !STREQ(dev->data.disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + dev->data.disk->driverName, dev->data.disk->src); + goto endjob; + } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { qemuReportError(VIR_ERR_INTERNAL_ERROR, -- 1.7.1
-- Shi Jin, Ph.D.
participants (4)
-
Daniel P. Berrange
-
Laine Stump
-
Shi Jin
-
Wen Congyang