Re: [Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int
On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: > Fix nbd_send_request to return int, as it returns a return value > of nbd_write (which is int), and the only user of nbd_send_request's > return value (nbd_co_send_request) consider it as int too. > > Signed-off-by: Vladimir Sementsov-Ogievskiy> --- > include/block/nbd.h | 2 +- > nbd/client.c| 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int
On Mon, Aug 07, 2017 at 11:57:12AM +0300, Vladimir Sementsov-Ogievskiy wrote: > 07.08.2017 11:23, Daniel P. Berrange wrote: > > On Fri, Aug 04, 2017 at 06:14:27PM +0300, Vladimir Sementsov-Ogievskiy > > wrote: > > > Fix nbd_send_request to return int, as it returns a return value > > > of nbd_write (which is int), and the only user of nbd_send_request's > > > return value (nbd_co_send_request) consider it as int too. > > The real problem here is the return value of nbd_write() - it should be > > a ssize_t, not an int, since it is propagating the return value of > > nbd_rwv() which is ssize_t. > > It was changed in f5d406fe86bb (sent in May) > The discussion actually was started half a year ago: > https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00825.html Actually I misread the code. nbd_rwv() is returning 0 on success, not the number of bytes, so int is in fact ok Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int
On 08/07/2017 03:57 AM, Vladimir Sementsov-Ogievskiy wrote: > 07.08.2017 11:23, Daniel P. Berrange wrote: >> On Fri, Aug 04, 2017 at 06:14:27PM +0300, Vladimir Sementsov-Ogievskiy >> wrote: >>> Fix nbd_send_request to return int, as it returns a return value >>> of nbd_write (which is int), and the only user of nbd_send_request's >>> return value (nbd_co_send_request) consider it as int too. >> The real problem here is the return value of nbd_write() - it should be >> a ssize_t, not an int, since it is propagating the return value of >> nbd_rwv() which is ssize_t. > > It was changed in f5d406fe86bb (sent in May) > The discussion actually was started half a year ago: > https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00825.html > > >> Basically all functions that do I/O and >> return the number of bytes read/written should be ssize_t - any use of >> int is a bug. If returning the size matters, then yes, not using ssize_t is a bug. But if all we need is to know whether an entire expected length was read (and treat ANY partial read the same as failure), then a mere int becomes enough to encode the results. Where I'm less certain is whether this change in semantics simplifies later patches, or loses information that might have been useful. But intuitively, ANY encounter of EOF means that NBD needs to quit trying to talk to the other end of the socket, whether or not the EOF occurred on a nice message boundary; and the rest of the protocol is strongly tied to messages, where we don't act until we have read the entire expected message; so if a later patch is indeed simpler by not returning bytes read, this patch might be okay. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int
07.08.2017 11:23, Daniel P. Berrange wrote: On Fri, Aug 04, 2017 at 06:14:27PM +0300, Vladimir Sementsov-Ogievskiy wrote: Fix nbd_send_request to return int, as it returns a return value of nbd_write (which is int), and the only user of nbd_send_request's return value (nbd_co_send_request) consider it as int too. The real problem here is the return value of nbd_write() - it should be a ssize_t, not an int, since it is propagating the return value of nbd_rwv() which is ssize_t. It was changed in f5d406fe86bb (sent in May) The discussion actually was started half a year ago: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00825.html Basically all functions that do I/O and return the number of bytes read/written should be ssize_t - any use of int is a bug. Regards, Daniel -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int
On Fri, Aug 04, 2017 at 06:14:27PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Fix nbd_send_request to return int, as it returns a return value > of nbd_write (which is int), and the only user of nbd_send_request's > return value (nbd_co_send_request) consider it as int too. The real problem here is the return value of nbd_write() - it should be a ssize_t, not an int, since it is propagating the return value of nbd_rwv() which is ssize_t. Basically all functions that do I/O and return the number of bytes read/written should be ssize_t - any use of int is a bug. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int
Fix nbd_send_request to return int, as it returns a return value of nbd_write (which is int), and the only user of nbd_send_request's return value (nbd_co_send_request) consider it as int too. Signed-off-by: Vladimir Sementsov-Ogievskiy--- include/block/nbd.h | 2 +- nbd/client.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index f7450608b4..040cdd2e60 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -163,7 +163,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, Error **errp); int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info, Error **errp); -ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request); +int nbd_send_request(QIOChannel *ioc, NBDRequest *request); int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp); int nbd_client(int fd); int nbd_disconnect(int fd); diff --git a/nbd/client.c b/nbd/client.c index a1758a1931..00cba45853 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -896,7 +896,7 @@ int nbd_disconnect(int fd) } #endif -ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request) +int nbd_send_request(QIOChannel *ioc, NBDRequest *request) { uint8_t buf[NBD_REQUEST_SIZE]; -- 2.11.1