Re: [PATCH v5 09/12] api: add virNodeDeviceUpdate()

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Tuesday in 2024, Boris Fiuczynski wrote:
> On 2/26/24 15:21, Ján Tomko wrote:
> > On a Thursday in 2024, Boris Fiuczynski wrote:
> > > A public API method which allows to update or modify objects is
> > > implemented for almost all other objects that have a concept of
> > > persistent definition and activatability. Currently node devices of type
> > > mdev can be persistent and active. This new method allows to update
> > > defined and active node devices as well.
> > >
> > > Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> > > ---
> > > include/libvirt/libvirt-nodedev.h | 18 +++++++++++++
> > > src/access/viraccessperm.c        |  1 +
> > > src/access/viraccessperm.h        |  6 +++++
> > > src/conf/virnodedeviceobj.c       | 42 +++++++++++++++++++++++++++++
> > > src/conf/virnodedeviceobj.h       |  3 +++
> > > src/driver-nodedev.h              |  6 +++++
> > > src/libvirt-nodedev.c             | 45 +++++++++++++++++++++++++++++++
> > > src/libvirt_private.syms          |  1 +
> > > src/libvirt_public.syms           |  5 ++++
> > > src/remote/remote_driver.c        |  1 +
> > > src/remote/remote_protocol.x      | 17 +++++++++++-
> > > src/remote_protocol-structs       |  6 +++++
> > > 12 files changed, 150 insertions(+), 1 deletion(-)
> > >
> > > static virNWFilterDriver nwfilter_driver = {
> > > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> > > index e295b0acc3..ffe865965d 100644
> > > --- a/src/remote/remote_protocol.x
> > > +++ b/src/remote/remote_protocol.x
> > > @@ -2237,6 +2237,12 @@ struct remote_node_device_is_active_ret {
> > >     int active;
> > > };
> > >
> > > +struct remote_node_device_update_args {
> > > +    remote_nonnull_string name;
> > > +    remote_nonnull_string xml_desc;
> > > +    unsigned int flags;
> > > +};
> > > +
> > >
> > > /*
> > >  * Events Register/Deregister:
> > > @@ -7021,5 +7027,14 @@ enum remote_procedure {
> > >      * @generate: both
> > >      * @acl: none
> > >      */
> > > -    REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446
> > > +    REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446,
> > > +
> > > +    /**
> > > +     * @generate: both
> > > +     * @priority: high
> >
> > Should this be high priority?
> >
> > REMOTE_PROC_NODE_DEVICE_DEFINE_XML is not, for example.
> > REMOTE_PROC_DOMAIN_DEFINE_XML_FLAGS is.
> >
> > > +     * @acl: node_device:write
> > > +     * @acl: node_device:save:!VIR_NODE_DEVICE_UPDATE_AFFECT_CONFIG|VIR_NODE_DEVICE_UPDATE_AFFECT_LIVE
> >
> > > +     * @acl: node_device:save:VIR_NODE_DEVICE_UPDATE_AFFECT_CONFIG
> >
> > Here, AFFECT_CONFIG requires the 'save' ACL, which seems pointless since
> > REMOTE_PROC_NODE_DEVICE_DEFINE_XML does not.
> >
> > Jano
>
> Jano,
> I was using REMOTE_PROC_NETWORK_UPDATE as a source since 
> virNetworkUpdate() was my model for the API.
>
> Should I remove
>     * @priority: high
> here or add it to REMOTE_PROC_NODE_DEVICE_DEFINE_XML?

Looking at other APIs, it seems we pick mostly randomly
and we're consistent at the inconsistence, so it does not matter here.

> Would adding
>     * @acl: node_device:save
> to REMOTE_PROC_NODE_DEVICE_DEFINE_XML be correct?

I think so, yes.

Jano

> > > +     */
> > > +    REMOTE_PROC_NODE_DEVICE_UPDATE = 447
> > > };
> >
> > _______________________________________________
> > Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
> > To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
>
> --
> Mit freundlichen Grüßen/Kind regards
>   Boris Fiuczynski
>
> IBM Deutschland Research & Development GmbH
> Vorsitzender des Aufsichtsrats: Wolfgang Wendt
> Geschäftsführung: David Faller
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx