On 10/15/13 11:41, Chen Hanxiao wrote:
-----Original Message----- From: Peter Krempa [mailto:pkrempa@redhat.com] Sent: Tuesday, October 15, 2013 4:58 PM On 10/15/13 05:54, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
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.
'readonly' is valid, but as that condition is only checking the prefix of the @mode string to be 'readonly' or 'shareable' then you can write anything after that and the check won't trigger. If you are going to use a comma separated list here, you need to tokenize the string and check every item in the array.
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)
If you make the argument check above bulletproof, which it isn't right now it will not bother you.
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);
Peter