[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH] block/vhdx: Use IEC binary prefixe
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH] block/vhdx: Use IEC binary prefixes from "qemu/units.h" |
Date: |
Mon, 25 Mar 2019 22:56:57 +0100 |
Le lun. 25 mars 2019 21:53, John Snow <address@hidden> a écrit :
>
>
> On 3/21/19 5:40 AM, Stefano Garzarella wrote:
> > IEC binary prefixes are defined in "qemu/units.h", so we
> > can remove redundant definitions in "block/vhdx.h".
> >
> > Signed-off-by: Stefano Garzarella <address@hidden>
> > ---
> > block/vhdx-log.c | 1 +
> > block/vhdx.c | 4 +++-
> > block/vhdx.h | 5 -----
> > 3 files changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/block/vhdx-log.c b/block/vhdx-log.c
> > index 3149ff08d8..ad9905eb6d 100644
> > --- a/block/vhdx-log.c
> > +++ b/block/vhdx-log.c
> > @@ -18,6 +18,7 @@
> > *
> > */
> > #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > #include "qapi/error.h"
> > #include "qemu-common.h"
> > #include "block/block_int.h"
> > diff --git a/block/vhdx.c b/block/vhdx.c
> > index b785aef4b7..e9b16c5ef6 100644
> > --- a/block/vhdx.c
> > +++ b/block/vhdx.c
> > @@ -16,6 +16,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > #include "qapi/error.h"
> > #include "block/block_int.h"
> > #include "block/qdict.h"
> > @@ -1889,7 +1890,8 @@ static int coroutine_fn
> vhdx_co_create(BlockdevCreateOptions *opts,
> > return -EINVAL;
> > }
> > if (block_size > VHDX_BLOCK_SIZE_MAX) {
> > - error_setg(errp, "Block size must not exceed %d",
> VHDX_BLOCK_SIZE_MAX);
> > + error_setg(errp, "Block size must not exceed %" PRId64,
> > + VHDX_BLOCK_SIZE_MAX);
>
> Oh, changing the constants changes the inferred type of this constant,
> correct?
>
Yes.
> > return -EINVAL;
> > }
> >
> > diff --git a/block/vhdx.h b/block/vhdx.h
> > index 1bfb4e4f73..deb6e362ad 100644
> > --- a/block/vhdx.h
> > +++ b/block/vhdx.h
> > @@ -18,11 +18,6 @@
> > #ifndef BLOCK_VHDX_H
> > #define BLOCK_VHDX_H
> >
> > -#define KiB (1 * 1024)
> > -#define MiB (KiB * 1024)
> > -#define GiB (MiB * 1024)
> > -#define TiB ((uint64_t) GiB * 1024)
> > -
>
> I'd include units.h here because there are other references to
> KiB/MiB/GiB and so on throughout the rest of this header file.
>
> I think it's bad form to have a header that cannot be included unless
> you include other headers manually alongside it.
>
Agreed.
> > #define DEFAULT_LOG_SIZE 1048576 /* 1MiB */
>
Can we also update this definition?
> /* Structures and fields present in the VHDX file */
> >
> >
>
>
>