[libvirt] [PATCH]virsh: support readonly in attach-disk command

[Daniel P. Berrange]

On Mon, Oct 14, 2013 at 04:48:18PM +0800, Osier Yang wrote:
> On 18/09/13 16:43, Chen Hanxiao wrote:
> >From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> >
> >support readonly in attach-disk virsh command
> >with option --readonly
> >
> >Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> >---
> >  tools/virsh-domain.c | 7 +++++++
> >  tools/virsh.pod      | 5 +++--
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> >diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> >index 3479a1c..d334ebe 100644
> >--- a/tools/virsh-domain.c
> >+++ b/tools/virsh-domain.c
> >@@ -315,6 +315,10 @@ static const vshCmdOptDef opts_attach_disk[] = {
> >       .type = VSH_OT_BOOL,
> >       .help = N_("shareable between domains")
> >      },
> >+    {.name = "readonly",
> >+     .type = VSH_OT_BOOL,
> >+     .help = N_("allow guest read-only access to disk")
> >+    },
> >      {.name = "rawio",
> >       .type = VSH_OT_BOOL,
> >       .help = N_("needs rawio capability")
> >@@ -612,6 +616,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> >      if (vshCommandOptBool(cmd, "shareable"))
> >          virBufferAddLit(&buf, "  <shareable/>\n");
> >+    if (vshCommandOptBool(cmd, "readonly"))
> >+        virBufferAddLit(&buf, "  <readonly/>\n");
> >+
> >      if (straddr) {
> >          if (str2DiskAddress(straddr, &diskAddr) != 0) {
> >              vshError(ctl, _("Invalid address."));
> >diff --git a/tools/virsh.pod b/tools/virsh.pod
> >index 0ae5178..91b4429 100644
> >--- a/tools/virsh.pod
> >+++ b/tools/virsh.pod
> >@@ -1908,8 +1908,8 @@ expected.
> >  [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
> >  [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>]
> >  [I<--type type>] [I<--mode mode>] [I<--config>] [I<--sourcetype soucetype>]
> >-[I<--serial serial>] [I<--wwn wwn>] [I<--shareable>] [I<--rawio>]
> >-[I<--address address>] [I<--multifunction>] [I<--print-xml>]
> >+[I<--serial serial>] [I<--wwn wwn>] [I<--shareable>] [I<--readonly>]
> >+[I<--rawio>] [I<--address address>] [I<--multifunction>] [I<--print-xml>]
> >  Attach a new disk device to the domain.
> >  I<source> is path for the files and devices. I<target> controls the bus or
> >@@ -1931,6 +1931,7 @@ I<cache> can be one of "default", "none", "writethrough", "writeback",
> >  "directsync" or "unsafe".
> >  I<serial> is the serial of disk device. I<wwn> is the wwn of disk device.
> >  I<shareable> indicates the disk device is shareable between domains.
> >+I<readonly> indicates the disk device is read-only.
> >  I<rawio> indicates the disk needs rawio capability.
> >  I<address> is the address of disk device in the form of pci:domain.bus.slot.function,
> >  scsi:controller.bus.unit or ide:controller.bus.unit.
> 
> Not that simple, there is an option "--mode", accepts both
> "shareable" and "readonly".
> also a standalone option "shareable", but I think it was a mistake.
> Though it won't
> cause problem since libvirt will ignore the multiple <shareable>
> tags when parsing
> the XML, if one specifies both --mode and --shareable like:
> 
> virsh attach-disk $dom --mode shareable --shareable ......
> 
> It will produce the the multiple <sharable> tags. I believe option
> --print-xml will
> tell the truth. T
> 
> I'm not clear why both "--mode" and "--shareable" were introduced,
> but apprrently
> we should avoid more conflicts now.
> 
> One could specify both "shareable" and "readonly" in the meantime,
> but  option
> "mode" doesn't support it yet. It's a problem. Though I don't like
> the "--mode",
> since its meaning is too broad.
> 
> About using which option to support "readonly", "--mode" or
> "--readonly", I don't
> have strong opinion. But anyway, we should get the existing
> conflicts solved first.

The '--mode' argument has been around since the command was first
added. The --shareable flag was mistakenly introduced later - it
should never have been added.  IMHO we should remove the documentation
for --shareable to discourage its use and not add any --readonly
flag.

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 :|