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

On 18/09/13 16:43, Chen Hanxiao wrote: >From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com&gt; > >support readonly in attach-disk virsh command >with option --readonly > >Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com&gt; >--- > 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.