[Libguestfs] [PATCH 1/2] New API: btrfs-image

Chen, Hanxiao chenhanxiao at cn.fujitsu.com
Fri Mar 6 02:26:51 UTC 2015



> -----Original Message-----
> From: Richard W.M. Jones [mailto:rjones at redhat.com]
> Sent: Thursday, March 05, 2015 8:47 PM
> To: Chen, Hanxiao/陈 晗霄
> Cc: libguestfs at redhat.com
> Subject: Re: [Libguestfs] [PATCH 1/2] New API: btrfs-image
> 
> On Tue, Mar 03, 2015 at 03:48:02AM -0500, Chen Hanxiao wrote:
> > Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> > +  if (compresslevel >= 0) {
> > +    snprintf (compresslevel_s, sizeof compresslevel_s, "%d", compresslevel);
> > +    ADD_ARG (argv, i, "-c");
> > +    ADD_ARG (argv, i, compresslevel_s);
> > +  }
> 
> Because compresslevel is an optional parameter, you can't just use it
> directly like this.  You have to check optargs_bitmask.  The condition
> should be something like:
> 
>   if ((optargs_bitmask & GUESTFS_BTRFS_IMAGE_COMPRESSLEVEL_BITMASK) &&
>       compresslevel >= 0) {
>     ...
>   }

Thanks for your comments. Will fix.
> 
> > +  if (numthreads) {
> 
> I would prefer not to expose the -t parameter, since it's essentially
> an implementation detail.
> 
> If btrfs-image doesn't do the right thing, then you could pass
> `sysconf (_SC_NPROCESSORS_ONLN)' here (see builder/pxzcat-c.c for an
> example).

btrfs-progs already did this kind of check for us.
For compresslevel is an optional parameter, leaving it there does not matter much.

> 
> > +This used to create an image of a btrfs filesystem.
> 
>      ^ This is used ...
> 
> > +All data will be zeroed, but metadata and the like is preserved." };
> 
> This is fine with the corrections above.
> 

Sure, will fix in v2.

Regards,
- Chen




More information about the Libguestfs mailing list