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.
--
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