[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v13 4/6] qapi: introduce PreallocMode and a new
From: |
Hu Tao |
Subject: |
Re: [Qemu-devel] [PATCH v13 4/6] qapi: introduce PreallocMode and a new PreallocMode full. |
Date: |
Wed, 3 Sep 2014 09:35:29 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Sep 02, 2014 at 03:51:23PM -0600, Eric Blake wrote:
> On 08/29/2014 02:33 AM, Hu Tao wrote:
> > This patch prepares for the subsequent patches.
> >
> > Signed-off-by: Hu Tao <address@hidden>
> > ---
> > block/qcow2.c | 23 +++++++++++++++--------
> > qapi/block-core.json | 16 ++++++++++++++++
> > tests/qemu-iotests/049.out | 2 +-
> > 3 files changed, 32 insertions(+), 9 deletions(-)
> >
>
> > @@ -1958,6 +1958,13 @@ static int qcow2_create(const char *filename,
> > QemuOpts *opts, Error **errp)
> > flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
> > }
> >
> > + if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
This one reads as 'support PREALLOC_MODE_METADATA' only,
>
> I find it a bit awkward that you are checking for PREALLOC_MODE_OFF
> implicitly ('prealloc &&') vs. checking for prealloc mode METADATA
> explicitly. Since there are only three modes, would it be any simpler
> to just have written:
>
> if (prealloc == PREALLOC_MODE_FULL) {
and this one reads as 'does't support PREALLOC_MODE_FULL'. Althrough
they are the same, but I'd prefer the former one. Anyway, the check is
removed in patch 6.
Regards,
Hu
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>