[libvirt] [PATCH v3 26/30] qemu_monitor_text: Catch IOMMU/VFIO related errors in qemuMonitorTextAddDrive

[Cole Robinson]

On 12/12/19 4:19 AM, Michal Privoznik wrote:
> On 12/10/19 9:50 PM, Cole Robinson wrote:
>> On 12/2/19 9:26 AM, Michal Privoznik wrote:
>>> Because this is a HMP we're dealing with, there is nothing like
>>> class of reply message, so we have to do some string comparison
>>> to guess if the command fails. Well, with NVMe disks whole new
>>> class of errors comes to play because qemu needs to initialize
>>> IOMMU and VFIO for them. You can see all the messages it may
>>> produce in qemu_vfio_init_pci().
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>   src/qemu/qemu_monitor_text.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
>>> index 9054682d60..6948a5bf90 100644
>>> --- a/src/qemu/qemu_monitor_text.c
>>> +++ b/src/qemu/qemu_monitor_text.c
>>> @@ -75,6 +75,13 @@ int qemuMonitorTextAddDrive(qemuMonitorPtr mon,
>>>           goto cleanup;
>>>       }
>>>   +    if (strstr(reply, "IOMMU") ||
>>> +        strstr(reply, "VFIO")) {
>>> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>>> +                       reply);
>>> +        goto cleanup;
>>> +    }
>>> +
>>>       ret = 0;
>>>      cleanup:
>>>
>>
>> For the code:
>>
>> Reviewed-by: Cole Robinson <crobinso at redhat.com>
>>
>> Does the blockdev infrastructure have magic that turns the Props into a
>> drive string? I don't see any -drive examples in the test output
> 
> Blockdev usues -device + -blockdev pair and obsoletes -drive. For instance:
> 
> libvirt.git $ grep "\-drive" $(git grep -l "\-blockdev" --
> tests/qemuxml2argvdata/)
> 
> Neither of -blockdev files has -drive.

Right. I think my confusion is that this is a pre-blockdev/-drive code
path, but I didn't see any explicit -drive handling in the series, just
-blockdev. So either I 1) missed the -drive changes but in that case
there should probably be test output demoing -drive output here, or 2)
this feature is -blockdev only in which case this is dead code. Or some
other case that I'm missing :)

- Cole