On 07/19/2017 11:43 AM, Vladimir Sementsov-Ogievskiy wrote: > 07.07.2017 23:30, Eric Blake wrote: > > [..] > >> >> +/* Returns -1 if NBD_OPT_GO proves the export @wantname cannot be >> + * used, 0 if NBD_OPT_GO is unsupported (fall back to NBD_OPT_LIST and >> + * NBD_OPT_EXPORT_NAME in that case), and > 0 if the export is good to >> + * go (with @info populated). */ >> +static int nbd_opt_go(QIOChannel *ioc, const char *wantname, >> + NBDExportInfo *info, Error **errp) >> +{
>> + if (reply.type == NBD_REP_ACK) { >> + /* Server is done sending info and moved into transmission >> + phase, but make sure it sent flags */ >> + if (len) { >> + error_setg(errp, "server sent invalid NBD_REP_ACK"); >> + nbd_send_opt_abort(ioc); > > server is already in transmission phase, so we cant send any options > more, shouldn't CMD_DISC be send here instead? > >> + return -1; >> + } >> + if (!info->flags) { >> + error_setg(errp, "broken server omitted >> NBD_INFO_EXPORT"); >> + nbd_send_opt_abort(ioc); > > and here For NBD_OPT_INFO, receipt of NBD_REP_ACK implies the server is still in negotiation phase; transmission phases is only technically possible on NBD_OPT_GO. That said, either error condition means the server is buggy, so it really doesn't matter what we do in response (we don't know if the server moved on to transmission phase or not, because it has already broken protocol by sending us garbage) - so trying to be courteous to tell the server we don't like their garbage vs. just silently disconnecting makes no real difference; even if the server doesn't like what we send (because it thinks we are out of phase), the server already broke protocol first. Either way, we're going to disconnect, and the server has to mop up after its own bugs. I don't think a followup patch is essential, but I'd also be okay with a patch that just eliminates the NBD_OPT_ABORT attempt and does a silent disconnect instead. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature