Re: [libvirt] [PATCH] qemu: Report cmdline output if VM dies early

On 04/30/2010 09:44 AM, Cole Robinson wrote: > qemuReadLogOutput early VM death detection is racy and won't always work. > Startup then errors when connecting to the VM monitor. This won't report > the emulator cmdline output which is typically the most useful diagnostic. > > Check if the VM has died at the very end of the monitor connection step, > and if so, report the cmdline output. > +static void > +qemuReadLogFD(int logfd, char *buf, int maxlen, int off) > +{ > + int ret; > + char *tmpbuf = buf+off; Isn't the style to separate operators by space? 'buf + off' > + > + ret = saferead(logfd, tmpbuf, maxlen-off-1); Likewise, 'maxlen - off - 1'. > + if (ret < 0) { > + ret = 0; > + } > + > + *(tmpbuf+ret) = '\0'; Stylistically, I like 'tmpbuf[ret]' better than '*(tmpbuf+ret)'.

> +} > + > static int > qemudWaitForMonitor(struct qemud_driver* driver, > virDomainObjPtr vm, off_t pos) > { > - char buf[4096]; /* Plenty of space to get startup greeting */ > + char buf[4096] = "\0"; /* Plenty of space to get startup greeting */ "" is sufficient; you don't have to use "\0" to zero-initialize a statically sized char[]. But overall, the patch looks sane; I didn't see any logic errors. ACK with the style nits addressed.