[libvirt] [PATCH] xen: Prevent updating device when attaching a device

[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?

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110208/6f7ecde8/attachment-0001.sig>