> -----Original Message----- > From: Peter Krempa [mailto:pkrempa@xxxxxxxxxx] > Sent: Tuesday, October 15, 2013 4:58 PM > On 10/15/13 05:54, Chen Hanxiao wrote: > > From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx> > > > > Current disk could accept both 'readonly' and 'shareable' > > at the same time. > > But '--mode' could only accept one parameter. > > > > This patch enables '--mode' accept parameters like examples below: > > > > virsh # attach-disk domain /home/1.img sdd --mode shareable,readonly > > > > if (mode) { > > - if (STRNEQ(mode, "readonly") && STRNEQ(mode, "shareable")) { > > + if (!(STRPREFIX(mode, "readonly") || > > + STRPREFIX(mode, "shareable"))) { > > This won't work if you use "--mode readonly,asdf". Also indentation of > the second line is off. If we use "--mode readonly,asdf", only 'readonly' will be recognized as valid parameter. > > > vshError(ctl, _("No support for %s in command > 'attach-disk'"), > > mode); > > goto cleanup; > > @@ -600,8 +601,23 @@ cmdAttachDisk(vshControl *ctl, const vshCmd > *cmd) > > (isFile) ? "file" : "dev", > > source); > > virBufferAsprintf(&buf, " <target dev='%s'/>\n", target); > > - if (mode) > > - virBufferAsprintf(&buf, " <%s/>\n", mode); > > + if (mode) { > > + if (STRPREFIX(mode, "readonly")) { > > + virBufferAddLit(&buf, " <readonly/>\n"); > > + } else { > > + virBufferAddLit(&buf, " <shareable/>\n"); > > + } > > + > > + char *rest; > > + if ((rest = strchr(mode, ','))) { > > + rest++; > > + if (STRPREFIX(rest, "readonly")) { > > + virBufferAddLit(&buf, " <readonly/>\n"); > > + } else if (STRPREFIX(rest, "shareable")) { > > + virBufferAddLit(&buf, " <shareable/>\n"); > > + } > > + } > > + } > > Hmmm, this is a very convoluted way to do stuff. I would recommend doing > the sanity check right and then you can do either: > > if (mode && > strstr(mode, "readonly")) > virBufferAddLit(&buf, " <readonly/>\n"); > > if (mode && > strstr(mode, "shareable")) > virBufferAddLit... > If we use strstr(), --mode XXshareableXX will take effect. I try to let --mode accept: (readonly as A, shareable as B) A B A,B[xxxx] B,A[xxxx] A,A[xxxx] B,B[xxxx] > or tokenize the string properly (see vshStringToArray) and output the > elements in a loop I will check this and see. Thanks for you hints. > > > > > if (serial) > > virBufferAsprintf(&buf, " <serial>%s</serial>\n", serial); > > diff --git a/tools/virsh.pod b/tools/virsh.pod > > index e12a800..f6a80a3 100644 > > --- a/tools/virsh.pod > > +++ b/tools/virsh.pod > > @@ -1915,7 +1915,7 @@ expected. > > =item B<attach-disk> I<domain> I<source> I<target> > > [[[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<--type type>] [I<--mode mode[,mode2]>] [I<--config>] [I<--sourcetype > soucetype>] > > [I<--serial serial>] [I<--wwn wwn>] [I<--shareable>] [I<--rawio>] > > [I<--address address>] [I<--multifunction>] [I<--print-xml>] > > > > > > also it might be worth mentioning the meaning of the option in the text > after this block. I'll add it in the future patch. > > Peter -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list