> > @@ -244,20 +232,18 @@ int vmbus_connect(void) > > * version. > > */ > > > > - version = VERSION_CURRENT; > > + for (i = 0; i < ARRAY_SIZE(vmbus_versions); i++) { > > + version = vmbus_versions[i]; > > > > - do { > > ret = vmbus_negotiate_version(msginfo, version); > > if (ret == -ETIMEDOUT) > > goto cleanup; > > > > if (vmbus_connection.conn_state == CONNECTED) > > break; > > + } > > > > - version = vmbus_get_next_version(version); > > - } while (version != VERSION_INVAL); > > - > > - if (version == VERSION_INVAL) > > + if (vmbus_connection.conn_state != CONNECTED) > > goto cleanup; > > > > This is a nit, but the loop exit path bugs me. When a connection > is established, the loop is exited by the "break", and then > conn_state has to be tested again to decide whether the loop > exited due to getting a connection vs. hitting the end of the list. > Slightly cleaner in my mind would be: > > for (i=0; ; i++) { > if (i == ARRAY_SIZE(vmbus_versions)) > goto cleanup; > > version = vmbus_versions[i]; > ret = vmbus_negotiate_version(msginfo, version); > if (ret == -ETIMEDOUT) > goto cleanup; > > if (vmbus_connection.conn_state == CONNECTED) > break; > } Indeed. I applied this locally, for the next iteration. Thank you for the review, Michael. Andrea