-----Original Message----- From: Peter Krempa [mailto:pkrempa@redhat.com] Sent: Tuesday, October 15, 2013 4:50 PM To: Chen Hanxiao; libvir-list@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@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