Richard W.M. Jones
2020-Feb-24 20:52 UTC
[Libguestfs] [PATCH commit] options: Compile blocksize code conditionally.
Since the blocksize option was added since the minimum version of libguestfs for virt-v2v, we need to make it conditional so that virt-v2v can still be compiled with the older libguestfs. This commit does the minimum fix to make it build. It doesn't conditionalize the --blocksize option altogether. However I believe, at least for virt-v2v, that is OK since the --blocksize option is not used there. Thanks: Pino Toscano Fixes: commit c33e0036c70ce68d40df92e4a6c0423e136e005c --- options/options.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/options/options.c b/options/options.c index 63221ea..abdcbae 100644 --- a/options/options.c +++ b/options/options.c @@ -140,10 +140,12 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK; ad_optargs.discard = drv->a.discard; } +#ifdef GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK if (drv->a.blocksize) { ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; ad_optargs.blocksize = drv->a.blocksize; } +#endif r = guestfs_add_drive_opts_argv (g, drv->a.filename, &ad_optargs); if (r == -1) @@ -177,10 +179,12 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; ad_optargs.secret = drv->uri.password; } +#ifdef GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK if (drv->uri.blocksize) { ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; ad_optargs.blocksize = drv->uri.blocksize; } +#endif r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs); if (r == -1) -- 2.24.1
Nikolay Ivanets
2020-Feb-25 08:32 UTC
Re: [Libguestfs] [PATCH commit] options: Compile blocksize code conditionally.
пн, 24 лют. 2020 о 22:53 Richard W.M. Jones <rjones@redhat.com> пише:> > Since the blocksize option was added since the minimum version of > libguestfs for virt-v2v, we need to make it conditional so that > virt-v2v can still be compiled with the older libguestfs. > > This commit does the minimum fix to make it build. It doesn't > conditionalize the --blocksize option altogether. However I believe, > at least for virt-v2v, that is OK since the --blocksize option is not > used there. > > Thanks: Pino Toscano > Fixes: commit c33e0036c70ce68d40df92e4a6c0423e136e005c > --- > options/options.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/options/options.c b/options/options.c > index 63221ea..abdcbae 100644 > --- a/options/options.c > +++ b/options/options.c > @@ -140,10 +140,12 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK; > ad_optargs.discard = drv->a.discard; > } > +#ifdef GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK > if (drv->a.blocksize) { > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; > ad_optargs.blocksize = drv->a.blocksize; > } > +#endif > > r = guestfs_add_drive_opts_argv (g, drv->a.filename, &ad_optargs); > if (r == -1) > @@ -177,10 +179,12 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; > ad_optargs.secret = drv->uri.password; > } > +#ifdef GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK > if (drv->uri.blocksize) { > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; > ad_optargs.blocksize = drv->uri.blocksize; > } > +#endif > > r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs); > if (r == -1) > -- > 2.24.1 >I see. That is because common is used as a sub-module in virt-v2v and bitmask flags comes from generated lib/guestfs.h. I didn't know that. But seems we have to remember that and it is hard to protect us from this to happen in the future. Or force developers to compile all modules dependent on common while making changes there. LGTM
Richard W.M. Jones
2020-Feb-25 08:37 UTC
Re: [Libguestfs] [PATCH commit] options: Compile blocksize code conditionally.
On Tue, Feb 25, 2020 at 10:32:13AM +0200, Nikolay Ivanets wrote:> пн, 24 лют. 2020 о 22:53 Richard W.M. Jones <rjones@redhat.com> пише: > > > > Since the blocksize option was added since the minimum version of > > libguestfs for virt-v2v, we need to make it conditional so that > > virt-v2v can still be compiled with the older libguestfs. > > > > This commit does the minimum fix to make it build. It doesn't > > conditionalize the --blocksize option altogether. However I believe, > > at least for virt-v2v, that is OK since the --blocksize option is not > > used there. > > > > Thanks: Pino Toscano > > Fixes: commit c33e0036c70ce68d40df92e4a6c0423e136e005c > > --- > > options/options.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/options/options.c b/options/options.c > > index 63221ea..abdcbae 100644 > > --- a/options/options.c > > +++ b/options/options.c > > @@ -140,10 +140,12 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) > > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK; > > ad_optargs.discard = drv->a.discard; > > } > > +#ifdef GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK > > if (drv->a.blocksize) { > > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; > > ad_optargs.blocksize = drv->a.blocksize; > > } > > +#endif > > > > r = guestfs_add_drive_opts_argv (g, drv->a.filename, &ad_optargs); > > if (r == -1) > > @@ -177,10 +179,12 @@ add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index) > > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; > > ad_optargs.secret = drv->uri.password; > > } > > +#ifdef GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK > > if (drv->uri.blocksize) { > > ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK; > > ad_optargs.blocksize = drv->uri.blocksize; > > } > > +#endif > > > > r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs); > > if (r == -1) > > -- > > 2.24.1 > > > > I see. That is because common is used as a sub-module in virt-v2v and > bitmask flags comes from generated lib/guestfs.h. > I didn't know that. > But seems we have to remember that and it is hard to protect us from > this to happen in the future.Yes the submodule structure is tricky, but the impact of the problem here wasn't large and it was easy to fix. Rich.> Or force developers to compile all modules dependent on common while > making changes there. > > LGTM-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Pino Toscano
2020-Feb-26 09:44 UTC
Re: [Libguestfs] [PATCH commit] options: Compile blocksize code conditionally.
On Monday, 24 February 2020 21:52:57 CET Richard W.M. Jones wrote:> Since the blocksize option was added since the minimum version of > libguestfs for virt-v2v, we need to make it conditional so that > virt-v2v can still be compiled with the older libguestfs. > > This commit does the minimum fix to make it build. It doesn't > conditionalize the --blocksize option altogether. However I believe, > at least for virt-v2v, that is OK since the --blocksize option is not > used there.Because of this, let's pull this in. Thanks, -- Pino Toscano
Maybe Matching Threads
- [common PATCH v4 0/1] options: add '--blocksize' option for C-based tools
- [common PATCH v3 0/1] options: add '--blocksize' option for C-based tools
- Re: [PATCH commit] options: Compile blocksize code conditionally.
- [common PATCH v2 0/1] options: add '--blocksize' option for C-based tools
- [common PATCH v2 1/1] options: add '--blocksize' option for C-based tools