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&gt; >> >> '--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.")); >