Re: [PATCH v2 09/11] api: add virNodeDeviceUpdate()

On 2/14/24 19:00, Jonathon Jongsma wrote: > On 2/14/24 6:59 AM, Boris Fiuczynski wrote: >> On 2/13/24 14:43, Boris Fiuczynski wrote: >>>> Tangentially related: >>>> >>>> Almost all of the other API objects can update their persistent >>>> definition from an XML file by calling the $(OBJECT)DefineXML >>>> command to replace the existing definition. The Node device API >>>> does not yet support doing that, maybe we should strive for a bit >>>> more API consistency here and implement that as well. >>>> >>>> Clearly the DefineXML() API will only operate on a persistent >>>> definition, so if we want to provide a way to update a live device, >>>> we'll need a new API, which is presumably where this one comes in... >>> >>> Correct. >>> I will take a look at how to align virNodeDeviceDefineXML >> >> The current libvirt code does not prevent to define an existing mdev >> and does call mdevctl which reports that mdevctl cowardly refuses to >> overwrite the existing config as an internal error. >> >> # virsh nodedev-define vfio-ccw-mdev-c60c_new-attr-set1.xml >> error: Failed to define node device from >> 'vfio-ccw-mdev-c60c_new-attr-set1.xml' >> error: internal error: Unable to define mediated device: Error: >> Cowardly refusing to overwrite existing config for >> 0.0.0008/c60cc60c-c60c-c60c-c60c-c60cc60cc60c >> >> Instead of handling this in libvirt by detecting on define if a >> persistent config already exists and instead of calling 'mdevctl >> define' using 'mdevctl modify' for the update should mdevctl become >> brave by removing its refusal? >> > > I don't think it would be a good idea to change the behavior of > mdevctl at this point. But even if we did change mdevctl behavior, > we'd still have to deal with older versions of mdevctl so we'd have to > handle it in libvirt anyway. > > Jonathon What would have to change in libvirt to handle the current versions not allowing to overwrite the config? The error above would remain for older medevctl versions.