[libvirt] [PATCH] update domain status forcibly even if attach a device failed
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82a2210..5dff64a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6638,7 +6638,11 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } - if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + /* update domain status forcibly because the domain status may be changed + * even if we attach the device failed. For example, a new controller may + * be created. + */ + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; endjob: -- 1.7.1
On 02/16/2011 08:32 PM, Wen Congyang wrote:
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml.
I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way. Should we be instead rolling back and undoing the new controller creation if adding the device fails? Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here? But, if others agree with this approach, then ACK to the code. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
At 02/18/2011 05:09 AM, Eric Blake Write:
On 02/16/2011 08:32 PM, Wen Congyang wrote:
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml.
I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way.
Should we be instead rolling back and undoing the new controller creation if adding the device fails?
We use qemuDomainFindOrCreateSCSIDiskController() to create scsi disk controller, so we do not know whether we create a new controller.
Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here?
Yes, it may be another way to solve this bug. But, it will cause too many saves(For example, if we attach a scsi which uses scsi disk controller 100, and we will create scsi disk controller 1 to 100)
But, if others agree with this approach, then ACK to the code.
At 02/18/2011 05:09 AM, Eric Blake Write:
On 02/16/2011 08:32 PM, Wen Congyang wrote:
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml.
I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way.
Should we be instead rolling back and undoing the new controller creation if adding the device fails?
Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here?
But, if others agree with this approach, then ACK to the code.
Ping... Anyone has a better way to solve this bug?
At 02/18/2011 05:09 AM, Eric Blake Write:
On 02/16/2011 08:32 PM, Wen Congyang wrote:
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml.
I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way.
Should we be instead rolling back and undoing the new controller creation if adding the device fails?
Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here?
But, if others agree with this approach, then ACK to the code.
We may forget this patch. Ping again Anyone has a better way to solve this bug?
On 02/17/2011 02:09 PM, Eric Blake wrote:
On 02/16/2011 08:32 PM, Wen Congyang wrote:
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml.
I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way.
Should we be instead rolling back and undoing the new controller creation if adding the device fails?
Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here?
But, if others agree with this approach, then ACK to the code.
Given the silence, and that this is a real bug fix, let's go ahead and commit this as-is, and we can do a followup patch later if anyone has a better idea. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
At 03/24/2011 03:26 AM, Eric Blake Write:
On 02/17/2011 02:09 PM, Eric Blake wrote:
On 02/16/2011 08:32 PM, Wen Congyang wrote:
Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml.
I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way.
Should we be instead rolling back and undoing the new controller creation if adding the device fails?
Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here?
But, if others agree with this approach, then ACK to the code.
Given the silence, and that this is a real bug fix, let's go ahead and commit this as-is, and we can do a followup patch later if anyone has a better idea.
OK. I have pushed this patch. Thanks.
participants (2)
-
Eric Blake -
Wen Congyang