[libvirt] [PATCH 1/4] backup: Prepare for Unix sockets in QMP nbd-server-start

[Eric Blake]

On 6/6/19 7:53 AM, Peter Krempa wrote:
> On Wed, Jun 05, 2019 at 22:01:07 -0500, Eric Blake wrote:
>> Migration always uses a TCP socket for NBD servers, because we don't
>> support same-host migration. But upcoming pull-mode incremental backup
>> needs to also support a Unix socket, for retrieving the backup from
>> the same host. Support this by plumbing virStorageNetHostDef through
>> the monitor calls, since that is a nice reusable struct that can track
>> both TCP and Unix sockets.
>>
>> Update qemumonitorjsontest to not only test the response to the
>> command, but to actually verify that the command itself uses the two
>> correct QMP forms.  I'm actually a bit surprised that we are not
>> utilizing qemuMonitorTestAddItemVerbatim more frequently.
>>
>> Signed-off-by: Eric Blake <eblake at redhat.com>
>> ---

>> +static int
>> +testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
>> +{
>> +    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr) opaque;
>> +    qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
> 
> I'd suggest you use qemuMonitorTestNewSchema here so that we preserve
> the schema checks.

Oh, I _completely_ missed what TestNewSchema was providing.

> 
>> +    int ret = -1;
>> +    virStorageNetHostDef server_tcp = {
>> +        .name = (char *)"localhost",
>> +        .port = 12345,
>> +        .transport = VIR_STORAGE_NET_HOST_TRANS_TCP,
>> +    };
>> +    virStorageNetHostDef server_unix = {
>> +        .socket = (char *)"/tmp/sock",
>> +        .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX,
>> +    };
>> +
>> +    if (!test)
>> +        return -1;
>> +
>> +    if (qemuMonitorTestAddItemVerbatim(test,
>> +                                       "{\"execute\":\"nbd-server-start\","
>> +                                       " \"arguments\":{\"addr\":{"
>> +                                       "  \"type\":\"inet\",\"data\":{"
>> +                                       "   \"host\":\"localhost\","
>> +                                       "   \"port\":\"12345\"}},"
>> +                                       "  \"tls-creds\":\"test-alias\"},"
>> +                                       " \"id\":\"libvirt-1\"}",
>> +                                       NULL, "{\"return\":{}}") < 0)

Testing for a verbatim response proves that we generated at least one
form of acceptable JSON, but if TestNewSchema is able to validate that
the entire command complies with the introspected schema advertised by
qemu, then that also serves to validate things, and with a lot less
fragility.  Yes, I'll fix that, now that I _finally_ understand what you
were asking for (you made a similar comment in your v8 review, and I
misunderstood it).

>> @@ -3036,6 +3089,7 @@ mymain(void)
> 
> ACK with schema checks preserved.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190606/c50bdf1c/attachment-0001.sig>