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

[Osier Yang]

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.

Osier