Re: [PATCH 2/2]virsh: remove attach-disk '--shareable' option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 15, 2013 at 05:09:22PM +0800, Chen Hanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: Peter Krempa [mailto:pkrempa@xxxxxxxxxx]
> > Sent: Tuesday, October 15, 2013 4:50 PM
> > To: Chen Hanxiao; libvir-list@xxxxxxxxxx
> > Subject: Re:  [PATCH 2/2]virsh: remove attach-disk '--shareable'
> option
> > 
> > On 10/15/13 05:54, Chen Hanxiao wrote:
> > > From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> > >
> > > '--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.

'--shareable' is completely pointless, but we cannot remove it
for backwards compatibility - only hide it from man pages and
help output.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]