[libvirt] [PATCH 2/2]virsh: remove attach-disk '--shareable' option
Peter Krempa
pkrempa at redhat.com
Tue Oct 15 09:18:24 UTC 2013
On 10/15/13 11:09, Chen Hanxiao wrote:
>
>
>> -----Original Message-----
>> From: Peter Krempa [mailto:pkrempa at redhat.com]
>> Sent: Tuesday, October 15, 2013 4:50 PM
>> To: Chen Hanxiao; libvir-list at redhat.com
>> Subject: Re: [libvirt] [PATCH 2/2]virsh: remove attach-disk '--shareable'
> option
>>
>> On 10/15/13 05:54, Chen Hanxiao wrote:
>>> From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
>>>
>>> '--mode' option could set shareable tag already.
>>> We do not need to duplicate options.
>>>
>>> },
>>> - {.name = "shareable",
>>> - .type = VSH_OT_BOOL,
>>> - .help = N_("shareable between domains")
>>> - },
>>
>> We can't remove this whole part. The correct approach is to use correct
>> flags to hide it.
>
> I checked enum ' Command Option Flags', it seems that we do not have
> an option for 'existed but not for use'.
> Could you please give me some hints for this?
Hmmm, right. There probably isn't a suitable option as VSH_OT_ALIAS has
to be used with the same type.
>
>>
>>> {.name = "rawio",
>>> .type = VSH_OT_BOOL,
>>> .help = N_("needs rawio capability")
>>> @@ -625,9 +621,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>>> if (wwn)
>>> virBufferAsprintf(&buf, " <wwn>%s</wwn>\n", wwn);
>>>
>>> - if (vshCommandOptBool(cmd, "shareable"))
>>> - virBufferAddLit(&buf, " <shareable/>\n");
>>> -
>>
>> NACK to this part. As DanPB mentioned, we need to remove the
>> documentation, to discourage use of it, but we can't remove the argument
>> as it would break backwards compatibility.
>>
>
> If --mode could do all the same job. I don't know why '--shareable'
> introduced.
I don't know either. But it was introduced and released already, thus we
can't kill it or we could possibly break someone who is already using it.
> One scenario I can imagine is to set <readonly/> and <shareable/> in one
> command.
>
> As Daniel mentioned, --shareable may be mistakenly introduced.
> I think at least we should remove it from docs and command --help.
> So we didn't lose compatibility and do the right way of using this command.
Well that's exactly what I was trying to say in my original review.
>
> Thanks
>
>>> if (straddr) {
>>> if (str2DiskAddress(straddr, &diskAddr) != 0) {
>>> vshError(ctl, _("Invalid address."));
>>
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131015/59c1f2af/attachment-0001.sig>
More information about the libvir-list
mailing list