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

[Osier Yang]

于 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