Re: [libvirt] [PATCH 2/2]virsh: remove attach-disk '--shareable' option
-----Original Message-----
From: Peter Krempa [mailto:pkrempa@redhat.com]
Sent: Tuesday, October 15, 2013 4:50 PM
To: Chen Hanxiao; libvir-list(a)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(a)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?
> {.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.
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.
Thanks
> if (straddr) {
> if (str2DiskAddress(straddr, &diskAddr) != 0) {
> vshError(ctl, _("Invalid address."));
Peter