Re: [Qemu-block] [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver

[Hailiang Zhang]

Hi,

On 2016/10/20 23:34, Eric Blake wrote:
> On 10/20/2016 08:57 AM, zhanghailiang wrote:
> > Without this callback, there will be an error reports in the primary side:
> > "qemu-system-x86_64: Couldn't determine the cluster size of the target image,
> > which has no backing file: Operation not supported
> > Aborting, since this may create an unusable destination image"
> >
> > For nbd driver, it doesn't have cluster size, so here we return
> > a fake value for it.
> >
> > Signed-off-by: zhanghailiang <address@hidden>
> > Signed-off-by: Wen Congyang <address@hidden>
> > ---
> >   block/nbd.c         | 12 ++++++++++++
> >   block/replication.c |  6 ++++++
> >   2 files changed, 18 insertions(+)
> >
> > diff --git a/block/nbd.c b/block/nbd.c
> > index 6bc06d6..96d7023 100644
> > --- a/block/nbd.c
> > +++ b/block/nbd.c
> > @@ -40,6 +40,8 @@
> >
> >   #define EN_OPTSTR ":exportname="
> >
> > +#define NBD_FAKE_CLUSTER_SIZE 512
>
> Why 512?  NBD allows byte-addressable operations (even if it is more
> efficient on aligned I/O); and I've been working hard to convert things
> to the point that NBD does not enforce alignment on other layers.
> Wouldn't 1 be better?

Yes, that makes no difference for block replication driver. :)

> > +static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> > +{
> > +    bdi->cluster_size  = NBD_FAKE_CLUSTER_SIZE;
>
> I also have patches written (but waiting for NBD write zeroes support to
> be reviewed first) that add support for the experimental NBD block info,
> that lets a server advertise actual sizes to the client rather than
> having to guess.  Here's the last time I posted a preview of it:
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg03567.html
>
> It would be nice to use that instead of just faking things.

That's great, that's what we really want, here it is just a
temporary solution, I'll drop this patch after you nbd patch been merged.

Thanks,
Hailiang