Re: [libvirt] [PATCHv4] libvirt: do not mix internal flags into public API
On 07/18/2011 02:53 PM, Laine Stump wrote:
On 07/15/2011 07:12 PM, Eric Blake wrote:
> There were two API in driver.c that were silently masking flags
> bits prior to calling out to the drivers, and several others
> that were explicitly masking flags bits. This is not
> forward-compatible - if we ever have that many flags in the
> future, then talking to an old server that masks out the
> flags would be indistinguishable from talking to a new server
> that can honor the flag. In general, libvirt.c should forward
> _all_ flags on to drivers, and only the drivers should reject
> unknown flags.
>
> +++ b/src/interface/netcf_driver.c
> @@ -344,6 +344,8 @@ static char *interfaceGetXMLDesc(virInterfacePtr
> ifinfo,
> virInterfaceDefPtr ifacedef = NULL;
> char *ret = NULL;
>
> + virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
> +
If I understand correctly, this is here to put the burden on the driver
to check/reject flags, rather than letting the client do it.
Correct.
Older drivers didn't reject unknown flags, so there is a situation where
a newer client talking to a pre-0.9.4 server will misbehave, but the
only way to fix that would be to teach 0.9.4 client codes to reject any
flags that did not exist prior to 0.9.4 before making the RPC.
But from 0.9.4 onwards, all drivers will all reject unknown flags, so
the RPC can blindly pass all flags through to let the remote driver
decide whether or not a flag is known, instead of rejecting the flag
prematurely.
> - ret = conn->secretDriver->getValue(secret, value_size,
flags);
> + ret = conn->secretDriver->getValue(secret, value_size, flags, 0);
So the getValue() callback has an extra internalFlags arg, but that is
only non-0 if get Value is called from the driver side (e.g. from qemu),
and never sent over the wire. A bit odd, but I can live with it (or you
can add the v5 interdiff - either way is fine with me).
I ended up pushing v4 as is, and saving the separation of driver
functions for a separate patch, which I'm still not convinced whether to
keep or drop.
ACK, either with or without the v5 interdiff added. Having a separate
callback seems cleaner, but may have other implications I'm not aware of.
Now pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org