[Libguestfs] [PATCH 07/10] Flexible guestfs_vmchannel parameter for future appliances.
Richard W.M. Jones
rjones at redhat.com
Mon Sep 21 22:14:51 UTC 2009
On Mon, Sep 21, 2009 at 10:08:50PM +0200, Jim Meyering wrote:
> > + static const char *options = "fc:?";
> > static const struct option long_options[] = {
> > + { "channel", 0, 0, 'c' },
>
> Shouldn't this be:
>
> { "channel", required_argument, 0, 'c' },
Yes, it should be :-(
> > { "foreground", 0, 0, 'f' },
> > { "help", 0, 0, '?' },
> > { 0, 0, 0, 0 }
> ...
> > + *
> > + * Sources:
> > + * --channel/-c option on the command line
> > + * guestfs_vmchannel=... from the kernel command line
> > + * guestfs=... from the kernel command line
> > + * built-in default
> > + *
> > + * At the moment we expect this to contain "tcp:ip:port" but in
> > + * future it might contain a device name, eg. "/dev/vcon4" for
> > + * virtio-console vmchannel.
> > + */
> > + if (vmchannel == NULL && cmdline) {
> > + char *p;
> > + int len;
>
> size_t len;
>
> > + p = strstr (cmdline, "guestfs_vmchannel=");
> > + if (p) {
> > + len = strcspn (p + 18, " \t\n");
> > + vmchannel = strndup (p + 18, len);
> > + if (!vmchannel) {
> > + perror ("strndup");
> > + exit (1);
> > + }
> > + }
> > +
> > + /* Old libraries passed guestfs=host:port. Rewrite it as tcp:host:port. */
> > + if (vmchannel == NULL) {
> > + p = strstr (cmdline, "guestfs=");
> > + if (p) {
> > + len = strcspn (p + 4, " \t\n");
> > + vmchannel = strndup (p + 4, len);
> > + if (!vmchannel) {
> > + perror ("strndup");
> > + exit (1);
> > + }
> > + memcpy (vmchannel, "tcp:", 4);
> > + }
>
> It took me a while to realize that the first two '4's above
> stand for strlen("guestfs=") - strlen("tcp:')
>
> Hoping to make it easier to follow, I wrote this:
>
> if (vmchannel == NULL) {
> p = strstr (cmdline, "guestfs=");
> if (p) {
> p += 8;
> len = strcspn (p, " \t\n");
> vmchannel = malloc (strlen("tcp:") + len + 1);
> if (!vmchannel) {
> perror ("malloc");
> exit (1);
> }
> stpcpy (stpncpy (stpcpy (vmchannel, "tcp:"), p, len), "");
> }
>
> I don't particularly like the literal 8
> or the duplicated "tcp:" strings, nor am I even
> sure which is the lesser of the evils ;-)
>
> And of course, if you're not used to the nested stpcpy idiom,
> that last line will look weird. Even if you are, it's probably not
> obvious that the outer stpcpy+"" arg is to NUL-terminate the result.
>
> So maybe just add a comment to yours.
I'll come up with a better patch tomorrow, and for the rest of the stuff
you mentioned below.
Thanks for looking at these patches.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
More information about the Libguestfs
mailing list