Skip to content
  • Eric Blake's avatar
    6e280648
    nbd/server: Trace client noncompliance on unaligned requests · 6e280648
    Eric Blake authored
    
    
    We've recently added traces for clients to flag server non-compliance;
    let's do the same for servers to flag client non-compliance. According
    to the spec, if the client requests NBD_INFO_BLOCK_SIZE, it is
    promising to send all requests aligned to those boundaries.  Of
    course, if the client does not request NBD_INFO_BLOCK_SIZE, then it
    made no promises so we shouldn't flag anything; and because we are
    willing to handle clients that made no promises (the spec allows us to
    use NBD_REP_ERR_BLOCK_SIZE_REQD if we had been unwilling), we already
    have to handle unaligned requests (which the block layer already does
    on our behalf).  So even though the spec allows us to return EINVAL
    for clients that promised to behave, it's easier to always answer
    unaligned requests.  Still, flagging non-compliance can be useful in
    debugging a client that is trying to be maximally portable.
    
    Qemu as client used to have one spot where it sent non-compliant
    requests: if the server sends an unaligned reply to
    NBD_CMD_BLOCK_STATUS, and the client was iterating over the entire
    disk, the next request would start at that unaligned point; this was
    fixed in commit a39286dd when the client was taught to work around
    server non-compliance; but is equally fixed if the server is patched
    to not send unaligned replies in the first place (yes, qemu 4.0 as
    server still has few such bugs, although they will be patched in
    4.1). Fortunately, I did not find any more spots where qemu as client
    was non-compliant. I was able to test the patch by using the following
    hack to convince qemu-io to run various unaligned commands, coupled
    with serving 512-byte alignment by intentionally omitting '-f raw' on
    the server while viewing server traces.
    
    | diff --git i/nbd/client.c w/nbd/client.c
    | index 427980bdd22..1858b2aac35 100644
    | --- i/nbd/client.c
    | +++ w/nbd/client.c
    | @@ -449,6 +449,7 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt,
    |                  nbd_send_opt_abort(ioc);
    |                  return -1;
    |              }
    | +            info->min_block = 1;//hack
    |              if (!is_power_of_2(info->min_block)) {
    |                  error_setg(errp, "server minimum block size %" PRIu32
    |                             " is not a power of two", info->min_block);
    
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    Message-Id: <20190403030526.12258-3-eblake@redhat.com>
    [eblake: address minor review nits]
    Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
    6e280648
    nbd/server: Trace client noncompliance on unaligned requests
    Eric Blake authored
    
    
    We've recently added traces for clients to flag server non-compliance;
    let's do the same for servers to flag client non-compliance. According
    to the spec, if the client requests NBD_INFO_BLOCK_SIZE, it is
    promising to send all requests aligned to those boundaries.  Of
    course, if the client does not request NBD_INFO_BLOCK_SIZE, then it
    made no promises so we shouldn't flag anything; and because we are
    willing to handle clients that made no promises (the spec allows us to
    use NBD_REP_ERR_BLOCK_SIZE_REQD if we had been unwilling), we already
    have to handle unaligned requests (which the block layer already does
    on our behalf).  So even though the spec allows us to return EINVAL
    for clients that promised to behave, it's easier to always answer
    unaligned requests.  Still, flagging non-compliance can be useful in
    debugging a client that is trying to be maximally portable.
    
    Qemu as client used to have one spot where it sent non-compliant
    requests: if the server sends an unaligned reply to
    NBD_CMD_BLOCK_STATUS, and the client was iterating over the entire
    disk, the next request would start at that unaligned point; this was
    fixed in commit a39286dd when the client was taught to work around
    server non-compliance; but is equally fixed if the server is patched
    to not send unaligned replies in the first place (yes, qemu 4.0 as
    server still has few such bugs, although they will be patched in
    4.1). Fortunately, I did not find any more spots where qemu as client
    was non-compliant. I was able to test the patch by using the following
    hack to convince qemu-io to run various unaligned commands, coupled
    with serving 512-byte alignment by intentionally omitting '-f raw' on
    the server while viewing server traces.
    
    | diff --git i/nbd/client.c w/nbd/client.c
    | index 427980bdd22..1858b2aac35 100644
    | --- i/nbd/client.c
    | +++ w/nbd/client.c
    | @@ -449,6 +449,7 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt,
    |                  nbd_send_opt_abort(ioc);
    |                  return -1;
    |              }
    | +            info->min_block = 1;//hack
    |              if (!is_power_of_2(info->min_block)) {
    |                  error_setg(errp, "server minimum block size %" PRIu32
    |                             " is not a power of two", info->min_block);
    
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    Message-Id: <20190403030526.12258-3-eblake@redhat.com>
    [eblake: address minor review nits]
    Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Loading