ä 2011å02æ09æ 01:06, Eric Blake åé:
On 02/08/2011 04:18 AM, Osier Yang wrote:
When attaching a device that already exists, xend driver updates
the device with "device_configure", it causes problems (e.g. for
disk device, 'device_configure' only can be used to update device
like CDROM), on the other hand, we provide additional API
(virDomainUpdateDevice) to update device, this fix is to raise up
errors instead of updating the existed device.
* src/xen/xend_internal.c
---
src/xen/xend_internal.c | 34 +++++++++++++++++++++++++++++-----
1 files changed, 29 insertions(+), 5 deletions(-)
The code makes sense to me. However, I'm worried that you've introduced
a regression in behavior, so you might want to wait for a second ACK.
@@ -4065,17 +4090,16 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
/* device doesn't exist, define it */
ret = xend_op(domain->conn, domain->name, "op", "device_create",
"config", sexpr, NULL);
- }
- else {
- /* device exists, attempt to modify it */
- ret = xend_op(domain->conn, domain->name, "op", "device_configure",
- "config", sexpr, "dev", ref, NULL);
This code dates back to Sep 2007 (commit fcf1b59), which was prior to
the creation of virDomainUpdateDevice (Mar 2010, commit 46a2ea3).
That is, any client that wanted to change CDROM had to use AttachDevice
between 2007 and Mar 2010, even though AttachDevice would corrupt a
non-CDROM device.
Can you rework the patch to decide between device_configure (for CDROM)
vs. error (for all other types), instead of blindly returning error, so
that existing uses of AttachDevice for CDROM alterations that pre-dated
UpdateDevice will still work?
Make sense, v2 will come. Thanks, :-)
Regards
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list