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. */ int 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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list