Re: [libvirt] [PATCH] qemu: add a value check for granularity

On 27.03.2015 10:56, Luyao Huang wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1206479 > > >From our manual of virsh and qemu side code, we know this > value must be power of 2, so instead of let qemu output error, > we can add a check when we file this value in qemuDomainBlockCopy. > > Signed-off-by: Luyao Huang <lhuang(a)redhat.com&gt; > --- > src/qemu/qemu_driver.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 2c55fb0..6d63317 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -16871,6 +16871,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, > } > bandwidth = param->value.ul; > } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_COPY_GRANULARITY)) { > + if (param->value.ui != VIR_ROUND_UP_POWER_OF_TWO(param->value.ui)) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("granularity must be power of 2")); > + goto cleanup; > + } > granularity = param->value.ui; > } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_COPY_BUF_SIZE)) { > buf_size = param->value.ul; > In fact, the virsh man page is not as precise as it could be either: diff --git a/tools/virsh.pod b/tools/virsh.pod index 63325ff..5d52761 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1039,10 +1039,10 @@ unlimited, but more likely would overflow; it is safer to use 0 for that purpose. Specifying I<granularity> allows fine-tuning of the granularity that will be copied when a dirty region is detected; larger values trigger less I/O overhead but may end up copying more data overall (the default value is usually correct); {+hypervisors may restrict+} this [-value must-]{+to+} be a power of [-two.-]{+two or fall+} {+within a certain range.+} Specifying I<buf-size> will control how much data can be simultaneously in-flight during the copy; larger values use more memory but may allow faster completion (the default value is usually correct). =item B<blockpull> I<domain> I<path> [I<bandwidth>] [I<base>] [I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]] I'm fixing the man page too, rewording the commit message a bit and ACKing. Will push after the release.

Michal