Re: [libvirt] [PATCH v3 3/3] qemu: Build command line for ivshmem device

On 09/26/2014 04:43 AM, Martin Kletzander wrote: > This patch implements support for the ivshmem device in QEMU. > > Signed-off-by: Maxime Leroy <maxime.leroy(a)6wind.com&gt; > Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > --- > + virBufferAddLit(&buf, "ivshmem"); > + if (shmem->size) { > + /* > + * Thanks to our parsing code, we have a guarantee that the > + * size is power of two and is at least a mebibyte in size. > + * But because it may change inthe future, the checks are s/inthe/in the/ > + * doubled in here. > + */ > + if (shmem->size & (shmem->size - 1)) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("shmem size must be a power of two")); > + goto error; > + } > + if (shmem->size < 1024 * 1024) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("shmem size must be at least 1 MiB")); > + goto error; > + } > + virBufferAsprintf(&buf, ",size=%llum", > + VIR_DIV_UP(shmem->size, 1024 * 1024)); Similar comment as before; since you already validated sizing and minimum value, you could use simpler >>20 instead of making me guess whether rounding up is occurring. > @@ -9897,7 +10014,6 @@ qemuBuildChrDeviceStr(char **deviceStr, > return ret; > } > > - > /* Spurious line deletion; you might as well restore the two lines between functions when doing the other touchups I've pointed out.