[libvirt] [PATCH 9/9] Modify virsh commands

[Jim Fehlig]

Daniel P. Berrange wrote:
> On Thu, Jan 14, 2010 at 10:42:46AM -0700, Jim Fehlig wrote:
>   
>> Change all virsh commands that invoke virDomain{Attach,Detach}Device()
>> to use virDomain{Attach,Detach}DeviceFlags() instead.
>>
>> Add a "--persistent" flag to these virsh commands, allowing user to
>> specify that the domain persisted config be modified as well.
>>     
>
>
>
>   
>> ---
>>  tools/virsh.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++------
>>  1 files changed, 49 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 1fae5e6..a082b89 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -6285,6 +6285,7 @@ static const vshCmdInfo info_attach_device[] = {
>>  static const vshCmdOptDef opts_attach_device[] = {
>>      {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")},
>>      {"file",   VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("XML file")},
>> +    {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist device attachment")},
>>      {NULL, 0, 0, NULL}
>>  };
>>  
>> @@ -6296,6 +6297,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
>>      char *buffer;
>>      int ret = TRUE;
>>      int found;
>> +    int flags = VIR_DOMAIN_DEVICE_MODIFY_CURRENT;
>>  
>>      if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
>>          return FALSE;
>> @@ -6309,13 +6311,18 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
>>          virDomainFree(dom);
>>          return FALSE;
>>      }
>> +    if (vshCommandOptBool(cmd, "persistent"))
>> +        flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
>>  
>>      if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
>>          virDomainFree(dom);
>>          return FALSE;
>>      }
>>  
>> -    ret = virDomainAttachDevice(dom, buffer);
>> +    if (virDomainIsActive(dom))
>> +       flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
>> +
>> +    ret = virDomainAttachDeviceFlags(dom, buffer, flags);
>>      VIR_FREE(buffer);
>>  
>>      if (ret < 0) {
>>     
>
>
> This has the same subtle compatability problem that the public API
> entry point suffers from. New virsh won't be able to modify guests
> from an existing libvirtd. I think that if flags == 0, then we should
> use the existing API method, and only use the new virDomainAttachDeviceFlags
> if flags != 0. I think we probably want to default to 0, and only set
> the VIR_DOMAIN_DEVICE_MODIFY_LIVE flag if a '--live' flag is given.
> Basically we need to try & ensure compatability with existing operation
> if at all possible
>   

The existing behavior is essentially VIR_DOMAIN_DEVICE_MODIFY_LIVE. 
qemu fails the operation if domain is inactive.  Attach works on
inactive Xen domains, but detach does not.  vbox has an impl for
inactive domains, but I haven't tested it.

I kept the existing behavior by only calling
vir{Attach,Detach}DeviceFlags() only when the new virsh flag
"persistent" is specified.  Revised patch below.

Thanks,
Jim

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 09.patch
Type: text/x-patch
Size: 9346 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100125/d91f30db/attachment-0001.bin>