| Message ID | 1401011408-8033-2-git-send-email-kroosec@gmail.com |
|---|---|
| State | New |
| Headers | show |
Il 25/05/2014 11:50, Hani Benhabiles ha scritto: > @@ -236,9 +236,10 @@ static int nbd_receive_options(NBDClient *client) > LOG("read failed"); > goto fail; > } > - TRACE("Checking reserved"); > - if (tmp != 0) { > - LOG("Bad reserved received"); > + TRACE("Checking client flags"); > + tmp = be32_to_cpu(tmp); > + if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) { > + LOG("Bad client flags received"); > goto fail; > } > > @@ -246,7 +247,7 @@ static int nbd_receive_options(NBDClient *client) > LOG("read failed"); > goto fail; > } > - TRACE("Checking reserved"); > + TRACE("Checking opts magic"); > if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) { > LOG("Bad magic received"); > goto fail; Here, if the client used "fixed new-style negotiation" you should reply with NBD_REP_ERR_UNSUP. You also need to turn this into a while loop. With these changes here, the logic in patch 2 should be okay. Thanks, Paolo
On Tue, May 27, 2014 at 04:46:46PM +0200, Paolo Bonzini wrote: > Il 25/05/2014 11:50, Hani Benhabiles ha scritto: > >@@ -236,9 +236,10 @@ static int nbd_receive_options(NBDClient *client) > > LOG("read failed"); > > goto fail; > > } > >- TRACE("Checking reserved"); > >- if (tmp != 0) { > >- LOG("Bad reserved received"); > >+ TRACE("Checking client flags"); > >+ tmp = be32_to_cpu(tmp); > >+ if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) { > >+ LOG("Bad client flags received"); > > goto fail; > > } > > > >@@ -246,7 +247,7 @@ static int nbd_receive_options(NBDClient *client) > > LOG("read failed"); > > goto fail; > > } > >- TRACE("Checking reserved"); > >+ TRACE("Checking opts magic"); > > if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) { > > LOG("Bad magic received"); > > goto fail; > > Here, if the client used "fixed new-style negotiation" you should reply with > NBD_REP_ERR_UNSUP. You also need to turn this into a while loop. With these > changes here, the logic in patch 2 should be okay. Hmm, seems that both the nbd-server implementation and the accompanying documentation use NBD_REP_ERR_UNSUP only when the NBD_OPT_* asked for by the client is unknown, not when the magic value is not NBD_OPTS_MAGIC. So what you are suggesting should actually happen a little below in the code. Thanks for the review. Will respin to add that and the loop behaviour.
diff --git a/include/block/nbd.h b/include/block/nbd.h index 79502a0..95d52ab 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -45,6 +45,12 @@ struct nbd_reply { #define NBD_FLAG_ROTATIONAL (1 << 4) /* Use elevator algorithm - rotational media */ #define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */ +/* New-style global flags. */ +#define NBD_FLAG_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */ + +/* New-style client flags. */ +#define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */ + #define NBD_CMD_MASK_COMMAND 0x0000ffff #define NBD_CMD_FLAG_FUA (1 << 16) diff --git a/nbd.c b/nbd.c index e5084b6..fb0a9cf 100644 --- a/nbd.c +++ b/nbd.c @@ -224,7 +224,7 @@ static int nbd_receive_options(NBDClient *client) int rc; /* Client sends: - [ 0 .. 3] reserved (0) + [ 0 .. 3] client flags [ 4 .. 11] NBD_OPTS_MAGIC [12 .. 15] NBD_OPT_EXPORT_NAME [16 .. 19] length @@ -236,9 +236,10 @@ static int nbd_receive_options(NBDClient *client) LOG("read failed"); goto fail; } - TRACE("Checking reserved"); - if (tmp != 0) { - LOG("Bad reserved received"); + TRACE("Checking client flags"); + tmp = be32_to_cpu(tmp); + if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) { + LOG("Bad client flags received"); goto fail; } @@ -246,7 +247,7 @@ static int nbd_receive_options(NBDClient *client) LOG("read failed"); goto fail; } - TRACE("Checking reserved"); + TRACE("Checking opts magic"); if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) { LOG("Bad magic received"); goto fail; @@ -333,6 +334,7 @@ static int nbd_send_negotiate(NBDClient *client) cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags); } else { cpu_to_be64w((uint64_t*)(buf + 8), NBD_OPTS_MAGIC); + cpu_to_be16w((uint16_t *)(buf + 16), NBD_FLAG_FIXED_NEWSTYLE); } if (client->exp) {
Signed-off-by: Hani Benhabiles <hani@linux.com> --- include/block/nbd.h | 6 ++++++ nbd.c | 12 +++++++----- 2 files changed, 13 insertions(+), 5 deletions(-)