Daniel P. Berrangé wrote: > On Mon, May 11, 2026 at 07:22:15PM +0200, Roman Bogorodskiy wrote: > > Implement QEMU Guest Agent support for bhyve. In bhyve it can configured > > using the virtio-console device. > > > > This change covers only two APIs using the agent: > > > > - DomainQemuAgentCommand -- the most generic one. > > - DomainGetHostname -- extended to support not only DHCP lease source, > > but an agent as well. > > > > There are two things that I'm not sure about this patch. > > > > - The protocol files were updated to make DomainQemuAgentCommand generic > > instead of Qemu specific. QEMU_PROC_DOMAIN_AGENT_COMMAND was removed in > > favor of REMOTE_PROC_DOMAIN_QEMU_AGENT_COMMAND. > > > > Even though the protocol is documented as private, I'm not sure how > > desired this change is. > > While we declare the extra protocols "Subject to change", we really > shouldn't change them without compelling reason, and I don't think > we have one. Either just add the new API in parallel with the old one, > or we could declare that the bhyve driver uses the libvirt-qemu.so > library for QEMU guest agent interaction and not change the protocol > at all ? Despite the name libvirt-qemu.so doesn't have a tight coupling > with thue QEMU driver.
It looks libvirt-qemu does not solve the issue. The main trigger for extending the protocol was the lack of ACL check in my bhyveDomainQemuAgentCommand() implementation. And as virDomainQemuAgentCommandEnsureACL() is available only in the generated access/viraccessapicheckqemu.h file, it does not seem there is a way I can use it. This leaves me with the new API option, I guess. > > - src/qemu/bhyve_qemu_agent.c and src/qemu/qemu_agent.c should > > share the same implementation. Ideally this should live > > somewhere in src/util/virqemuagent.c, but as it is using things > > from conf/, such as virDomainObj, it cannot be moved as is. > > > > I considered extracting a simpler data structure that will not > > use the conf/ types, but it didn't work very well for me and > > I didn't like the way it looks in the driver code. > > > > Maybe I'm missing some better approach? > > We have "src/hypervisor/" which can be used in these scenarios. > Thanks, I will take a look! > With regards, > Daniel > -- > |: https://berrange.com ~~ https://hachyderm.io/@berrange :| > |: https://libvirt.org ~~ https://entangle-photo.org :| > |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :| >
