Re: [Libguestfs] [PATCH nbdkit] Minimal implementation of NBD Structured Replies.
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/