[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/19] nbd/server: get rid of nbd_negotiate_read
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 01/19] nbd/server: get rid of nbd_negotiate_read and friends |
Date: |
Wed, 31 May 2017 09:39:27 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 05/31/2017 08:12 AM, Vladimir Sementsov-Ogievskiy wrote:
> 30.05.2017 23:10, Eric Blake wrote:
>> On 05/30/2017 09:30 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Functions nbd_negotiate_{read,write,drop_sync} were introduced in
>>> 1a6245a5b, when nbd_wr_syncv was working through qemu_co_sendv_recvv,
>> There is no qemu_co_sendv_recvv. Did you mean qemu_co_recv/qemu_co_send?
>
> qemu_co_recv is a macro, qemu_co_sendv_recvv - is an actual function,
> which do something.
Oh. I see where I went wrong - I was grepping for 'nbd_co_sendv' and
coming up short.
>
> #define qemu_co_recv(sockfd, buf, bytes) \
> qemu_co_send_recv(sockfd, buf, bytes, false)
>
> ssize_t coroutine_fn
> qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool do_send)
> {
> struct iovec iov = { .iov_base = buf, .iov_len = bytes };
> return qemu_co_sendv_recvv(sockfd, &iov, 1, 0, bytes, do_send);
> }
The commit message still makes me chase through several layers of
indirection, so I still wonder if referring to qemu_co_recv/qemu_co_send
(which is what is directly used in that older commit for nbd_wr_syncv)
is any better. It is probably also worth referring back to commit
ff82911cd as the point in time where we switched to the qio_channel
code, thereby no longer needing to manage coroutine handlers quite as
directly as beforehand.
>>> +int drop_sync(QIOChannel *ioc, size_t size, Error **errp)
>> As part of moving it into nbd/common.c, please rename this function to
>> an nbd_ prefix, since non-static functions are more likely to collide
>> with the rest of the code base if not properly named. Better yet: do it
>> in multiple patches:
>> - rename the static functions and fallout to callers (all of
>> nbd_drop_sync, nbd_read_sync, and nbd_write_sync)
In fact, does the _sync name buy us anything any more, or can we just go
with the shorter nbd_drop(), nbd_read(), and nbd_write()?
>> - code motion (promote nbd_drop_sync from static in client.c to exported
>> in common.c)
>> - drop nbd_negotiate_ functions in favor of common nbd_*_sync functions
>
> OK
>
>>
>> The idea makes sense, but I think there's too much going on in this one
>> commit.
>>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 18/19] nbd/client: refactor TRACE of NBD_MAGIC, (continued)
- [Qemu-devel] [PATCH 18/19] nbd/client: refactor TRACE of NBD_MAGIC, Vladimir Sementsov-Ogievskiy, 2017/05/30
- [Qemu-devel] [PATCH 14/19] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT, Vladimir Sementsov-Ogievskiy, 2017/05/30
- [Qemu-devel] [PATCH 02/19] nbd/server: get rid of ssize_t, Vladimir Sementsov-Ogievskiy, 2017/05/30
- [Qemu-devel] [PATCH 17/19] nbd/common: nbd_tls_handshake: use error_reportf_err instead of TRACE, Vladimir Sementsov-Ogievskiy, 2017/05/30
- [Qemu-devel] [PATCH 06/19] nbd/server: remove NBDClientNewData, Vladimir Sementsov-Ogievskiy, 2017/05/30
- [Qemu-devel] [PATCH 01/19] nbd/server: get rid of nbd_negotiate_read and friends, Vladimir Sementsov-Ogievskiy, 2017/05/30
[Qemu-devel] [PATCH 15/19] nbd/server: use errp instead of LOG, Vladimir Sementsov-Ogievskiy, 2017/05/30
[Qemu-devel] [PATCH 16/19] nbd/server: add errp to nbd_send_reply(), Vladimir Sementsov-Ogievskiy, 2017/05/30
[Qemu-devel] [PATCH 03/19] nbd/server: refactor nbd_co_send_reply, Vladimir Sementsov-Ogievskiy, 2017/05/30
[Qemu-devel] [PATCH 04/19] nbd/server: get rid of EAGAIN dead code, Vladimir Sementsov-Ogievskiy, 2017/05/30
[Qemu-devel] [PATCH 19/19] nbd: use generic trace subsystem instead of TRACE macro, Vladimir Sementsov-Ogievskiy, 2017/05/30