Re: [PATCH v2 REPOST 4/8] Qemu Monitor API entry point.

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

 



On 07/01/10 - 04:02:42PM, Eric Blake wrote:
> On 07/01/2010 02:59 PM, Chris Lalancette wrote:
> > Add the library entry point for the new virDomainQemuMonitorCommand()
> > entry point.  Because this is not part of the "normal" libvirt API,
> > it gets it's own header file, library file, and will eventually
> > get it's own over-the-wire protocol later in the series.
> 
> s/it's/its/

Oops, I'll fix that :).

> 
> > +
> > +int
> > +virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd,
> > +                            char **result, unsigned int flags)
> > +{
> > +    virConnectPtr conn;
> > +
> > +    DEBUG("domain=%p, cmd=%s, result=%p, flags=%u", domain, cmd, result, flags);
> 
> Do we want to use virCheckFlags(0, -1) here?

Again, this is a non-standard use of virDomainQemuMonitorCommand.  I'm actually
less against its use here, since this is an entry point with a user-provided
flag parameter, but it's not standard practice at present to add it to these
entry points.

Indeed, in the general case it *has* to be pushed to the
individual driver entry points, because we don't know what flags individual
drivers support.  The best we could do is to do the check twice; at the libvirt
entry point, we could check that it is one of the valid flags, and then at the
driver entry point, check that it is one of the flags this driver supports.
The benefit to this scheme is that we can catch gratuitious errors early
without doing an RPC call; the downside is that we are repeating the
virCheckFlags() call twice.

In any case, I think that would be a separate patch, so I would argue to leave
this as-is for now (minus my grammatical mistake :).

--
Chris Lalancette

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