Re: [libvirt] [PATCH] virsh: A bit smarter attach-disk

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;