[Libguestfs] [PATCH nbdkit] Minimal implementation of NBD Structured Replies.

[Richard W.M. Jones]

On Fri, Mar 08, 2019 at 09:03:05AM -0600, Eric Blake wrote:
> On 3/8/19 4:04 AM, Richard W.M. Jones wrote:
> > Also we do not understand NBD_CMD_FLAG_DF, but that seems to be OK
> > because (a) we don't advertize the feature and (b) we only send back a
> > single chunk anyway.
> 
> Or, we COULD advertise it because we always honor it (but that's a
> larger diffstat, and thus at odds with "minimal implementation"). Either
> way works.

What's also unclear to me is how NBD_CMD_FLAG_DF interacts with
NBD_CMD_BLOCK_STATUS.  What does it mean for extents which are by
their nature fragmented?

> > +/* Structured reply types. */
> > +extern const char *name_of_nbd_reply_type (int);
> > +#define NBD_REPLY_TYPE_NONE         0
> > +#define NBD_REPLY_TYPE_OFFSET_DATA  1
> > +#define NBD_REPLY_TYPE_OFFSET_HOLE  2
> > +#define NBD_REPLY_TYPE_BLOCK_STATUS 3
> > +#define NBD_REPLY_TYPE_ERROR        32769
> > +#define NBD_REPLY_TYPE_ERROR_OFFSET 32770
> 
> Worth writing these later ones in hex or via a helper macro that does
> ((1 << 15) | value)? Or would that mess up the generated
> protocol-to-lookup magic?

Could do it either way really.  The sed magic uses the symbol
(eg. NBD_REPLY_TYPE_ERROR) not the value so either should work.  I'll
play around with it to see which looks nicer.

> > +  reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC);
> > +  reply.handle = handle;
> > +  reply.flags = htobe16 (NBD_REPLY_FLAG_DONE);
> > +  reply.type = htobe16 (NBD_REPLY_TYPE_OFFSET_DATA);
> > +  reply.length = htobe32 (sizeof offset_data + count);
> 
> This line is correct, but I had to remind myself of C precedence rules
> on this one; writing 'count + sizeof offset_data' instead has the same
> effect without worrying whether sizeof binds with higher or lower
> precedence than +.

Yup I'll change this.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/