On 15.03.2012 10:13, Osier Yang wrote:
> Detects the file type of source path if no "--sourcetype" and
> "driver" is specified, instead of always set the disk type as
> "block".
>
> And previous "virCommandOptString" ensures "source" is not NULL,
> remove the conditional checking.
> ---
> tools/virsh.c | 14 +++++++++-----
> 1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index d45a4c9..3b845ac 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -14428,6 +14428,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> const char *stype = NULL;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> char *xml;
> + struct stat st;
>
> if (!vshConnectionUsability(ctl, ctl->conn))
> goto cleanup;
> @@ -14458,8 +14459,12 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> }
>
> if (!stype) {
> - if (driver && (STREQ(driver, "file") || STREQ(driver, "tap")))
> + if (driver && (STREQ(driver, "file") || STREQ(driver, "tap"))) {
> isFile = true;
> + } else {
> + if (!stat(source, &st))
> + isFile = S_ISREG(st.st_mode) ? true : false;
I think S_ISREG() would be sufficient here. But that's just cosmetic.
> + }
> } else if (STREQ(stype, "file")) {
> isFile = true;
> } else if (STRNEQ(stype, "block")) {
> @@ -14497,10 +14502,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> if (driver || subdriver || cache)
> virBufferAddLit(&buf, "/>\n");
>
> - if (source)
> - virBufferAsprintf(&buf, " <source %s='%s'/>\n",
> - (isFile) ? "file" : "dev",
> - source);
> + virBufferAsprintf(&buf, " <source %s='%s'/>\n",
> + (isFile) ? "file" : "dev",
> + source);
> virBufferAsprintf(&buf, " <target dev='%s'/>\n", target);
> if (mode)
> virBufferAsprintf(&buf, " <%s/>\n", mode);
However this looks bad. As written in commend just below
virCommandOptString(, source):
/* Allow empty string as a placeholder that implies no source, for
* use in adding a cdrom drive with no disk. */
if (!*source)
source = NULL;
That means:
virsh attach-disk <domain> "" dummy
make source NULL. Therefore you want to check source != NULL in the
first chunk too (okay, not as strict as here, since passing NULL to
stat() makes it fail, but it's clean coding style what matters too).
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list