Re: [PATCH 1/5]virsh: disable config readonly and shareable in virsh command

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 18.10.2013 07:42, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> 
> Daneil's suggestion about flag <shareable/> and <readonly/> as follow:
> - Exclusive read-write. This is the default
> - Shared read-write. This is the <shareable/> flag
> - Shared read-only. This is the <readonly/> flag
> 
> So we should disable config both readonly and shareable
> in virsh command to solve the confliction.
> For backwards compatibility we keep the code about '<shareable/>'.
> In this patch, '--mode' option will have high priority.
> If set, it will screen the '--shareable' option.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> ---
>  tools/virsh-domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 6d241db..2aed9f9 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -609,7 +609,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>      if (wwn)
>          virBufferAsprintf(&buf, "  <wwn>%s</wwn>\n", wwn);
>  
> -    if (vshCommandOptBool(cmd, "shareable"))
> +    if (!mode && vshCommandOptBool(cmd, "shareable"))
>          virBufferAddLit(&buf, "  <shareable/>\n");
>  
>      if (straddr) {
> 

So IIUC, it's still possible to use '--mode readonly' and '--shareable' at the same time (of course, the latter one won't get applied, but no error is thrown either).

I think, we should make use of '--mode' and '--shareable' exclusive. So we need something like this:

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b75f331..30fadce 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -602,8 +602,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
     if (wwn)
         virBufferAsprintf(&buf, "  <wwn>%s</wwn>\n", wwn);
 
-    if (vshCommandOptBool(cmd, "shareable"))
+    if (vshCommandOptBool(cmd, "shareable")) {
+        if (mode) {
+            vshError(ctl, "%s", _("--shareable and --mode are mutually exclusive"));
+            goto cleanup;
+        }
         virBufferAddLit(&buf, "  <shareable/>\n");
+    }
 
     if (straddr) {
         if (str2DiskAddress(straddr, &diskAddr) != 0) {


Michal

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