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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
>> ---

>> +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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux