[virtio] scsi: Correctly handle command completion

Correct a bug in how ExecuteCommandSync called IrqRingUpdate that would
cause the scan worker thread to wait forever.

virtio ring::IrqRingUpdate will invoke a passed-in functor on descriptor
chains completed by the device. However if no descriptor chains were
completed, it would not call the functor. (I had assumed at first skim
that it would have waited instead).

Modify ExecuteCommandSync to keep invoking IrqRingUpdate with 5 mS steps
until the command completes with either an error or success. Later we will
wire up interrupt mode support, to avoid this 5 mS polling looping.

ZX-2314

Tested:
1) Booted on GCE, dm dump displayed the virtio-scsi target at 1-0.

2) Still boots on QEMU (qemu masked the error by synchronously processing
   control commands).

            [00:02.0] pid=2006 /boot/driver/bus-pci.so
               <00:02.0> pid=2884 /boot/driver/bus-pci.proxy.so
                  [virtio-scsi] pid=2884 /boot/driver/virtio.so
                     [scsi-disk-1-1] pid=2884 /boot/driver/virtio.so

Both QEMU and GCE tests need the LUN encoding fix included (see 239313) for
tests to work.

Change-Id: I9a0d8f5860f11cf1f4b900ebe3c5c4cf76a060cd

system/dev/bus/virtio/scsi.cpp

diff --git a/system/dev/bus/virtio/scsi.cpp b/system/dev/bus/virtio/scsi.cpp
index a338864..9e7e06f 100644
--- a/system/dev/bus/virtio/scsi.cpp
+++ b/system/dev/bus/virtio/scsi.cpp
@@ -68,26 +68,32 @@
 
     // Wait for request to complete.
     sync_completion_t sync;
-    // annotalysis is unable to determine that ScsiDevice::lock_ is held when the IrqRingUpdate
-    // lambda is invoked.
-    request_queue_.IrqRingUpdate([this, &sync](vring_used_elem* elem) TA_NO_THREAD_SAFETY_ANALYSIS {
-        auto index = static_cast<uint16_t>(elem->id);
+    for (;;) {
+        // annotalysis is unable to determine that ScsiDevice::lock_ is held when the IrqRingUpdate
+        // lambda is invoked.
+        request_queue_.IrqRingUpdate(
+            [this, &sync](vring_used_elem* elem) TA_NO_THREAD_SAFETY_ANALYSIS {
+            auto index = static_cast<uint16_t>(elem->id);
 
-        // Synchronously reclaim the entire descriptor chain.
-        for (;;) {
-            vring_desc const* desc = request_queue_.DescFromIndex(index);
-            const bool has_next = desc->flags & VRING_DESC_F_NEXT;
-            const uint16_t next = desc->next;
+            // Synchronously reclaim the entire descriptor chain.
+            for (;;) {
+                vring_desc const* desc = request_queue_.DescFromIndex(index);
+                const bool has_next = desc->flags & VRING_DESC_F_NEXT;
+                const uint16_t next = desc->next;
 
-            this->request_queue_.FreeDesc(index);
-            if (!has_next) {
-                break;
+                this->request_queue_.FreeDesc(index);
+                if (!has_next) {
+                    break;
+                }
+                index = next;
             }
-            index = next;
+            sync_completion_signal(&sync);
+        });
+        auto status = sync_completion_wait_deadline(&sync, ZX_MSEC(5));
+        if (status == ZX_OK) {
+            break;
         }
-        sync_completion_signal(&sync);
-    });
-    sync_completion_wait(&sync, ZX_TIME_INFINITE);
+    }
 
     // If there was either a transport or SCSI level error, return a failure.
     if (resp->response || resp->status) {