[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension
From: |
Wouter Verhelst |
Subject: |
Re: [Qemu-devel] [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension |
Date: |
Thu, 24 Mar 2016 13:31:42 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Thu, Mar 24, 2016 at 12:37:33PM +0100, Paolo Bonzini wrote:
>
>
> On 24/03/2016 09:26, Wouter Verhelst wrote:
> >> >
> >> > No, there is no specific reason. Looks like NBD_CMD_FLAG_ZEROES fits the
> >> > spec and implementations nicely. So I'll rewrite the extension and add
> >> > the flag instead of the whole command.
> > Actually, having given this some more thought...
> >
> > There is at least one server-side implementation of nbd (mine) which
> > silently ignores flags it doesn't know about. This isn't a problem for
> > non-critical flags, but it could be a problem for a flag like this. Of
> > course, a client shouldn't send a flag to a server which that server
> > hasn't heard of, but mistakes do happen.
> >
> > Do we want to keep that in mind? If so, we might want to keep it as a
> > separate command after all.
> >
> > OTOH, it could be said that silently ignoring unknown messages is a bug.
> > I should probably just fix my implementation instead.
>
> Even if it is a bug, it does suggest that the payload format should not
> be changed by flags. For example ignoring flags is a bug for an NBD
> server, but not for a Wireshark protocol dissector.
Agreed. Let's make this a different command then, instead.
--
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12
- [Qemu-devel] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension, (continued)
[Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Denis V. Lunev, 2016/03/23
Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Wouter Verhelst, 2016/03/23