On Thu, Jun 06, 2019 at 08:40:14 -0500, Eric Blake wrote:
> 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.
It has one catch though that I've figured out now.
qemuMonitorTestAddItemVerbatim actually does not support schema checking
yet. I've implemented only a limited subset of the internals to support
it.
Probably the best course of actions for this test will be to swithch it
to qemuMonitorTestAddItem which does schema checking.
In this case I feel it's more useful to do the check against the schema
rather than to see that the resposne is the same.
Alternatively I can see whether it's reasonably feasible to do schema
checking also in qemuMonitorTestAddItemVerbatim.
> >> + 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
Well, it still validates only the one given syntax at this point,
because there's no sane way to compare everything what libvirt is able
to generate.
> 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).
The huge advantage of this is that I don't actually have to check for
typos and other things manually against qemu's schema :)
>
> >> @@ -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
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
