[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2] Try stdin for input when no file is specified

On 06/14/2011 12:12 AM, Michael Williams wrote:
> isatty() produces the correct behavior in all cases but the first one
> you mentioned, ie running "virsh define", which will not wait for input
> with the patch as it is.
> I see the ctl->imode parameter used in main() to determine the
> interactivity of the session, though that is only accessible at the
> cmdOPERATION level, not in virFileReadAll().  Maybe (yet) another read
> wrapper local to virsh.c that would check the ctl->imode if no path is
> offered, and make a decision from there.

Yeah, that seems best.  Basically, a function in virsh that does
something like (untested):

/* If the named option is present, read that file into
 * buffer; if the option is not present, then decide whether
 * to read stdin or give an error.
vshFileRead(vshControl *ctl, const vshCmd *cmd,
            const char *option, char **buffer)
    char *file = NULL;
    int ret = vshCommandOptString(cmd, option, &file);
    if (ret < 0) {
        vshError(ctl, _("option %s must not be empty"), option);
        return ret;
    if (ret == 0) {
        if (ctl->lastCommand || (ctl->imode && isatty(STDIN_FILENO))) {
            file = "-";
        } else {
            vshError(ctl, _("refusing to use implicit stdin"));
            return -1;
    return virFileReadAll(file, limit, buffer);

and which would replace this code:

 -    if (vshCommandOptString(cmd, "file", &from) <= 0)
+    if (vshCommandOptString(cmd, "file", &from) < 0) {
+        vshError(ctl, "%s", _("malformed xml argument"));
         return false;
+    }
      if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
         return false;

with the simpler:

if (vshFileRead(ctl, cmd, "file", &buffer) < 0)
    return false;

That proposal also implies adding a new ctl->lastCommand flag, which
tracks whether a given command is the last command given to virsh.  That
flag would default to false, and be set to true when you use the
one-command 'virsh command arg' form, or when you use the multi-command
'virsh "command; command"' form and the parser detects the end of the
input string before issuing the last command.

Hmm, looking at my above example, it looks like it is indeed easier to
put the "-" logic into virFileReadAll, after all, but that
virFileReadAll must always be given a non-NULL filename (that is, the
conversion from NULL to "-" is done in virsh, but the conversion from
"-" to stdin is done in util).

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]