Re: [PATCH 0/6] Introduce OpenSSH authorized key file mgmt APIs

On Thu, Nov 12, 2020 at 12:55:23 +0100, Michal Privoznik wrote: Yeah, using virDomainGetGuestInfo as the getter for the keys will not work. What I ultimately want to suggest is something more generic which will allow driving guest agent APIs libvirt doesn't actually care about without having to add single-use APIs for every single one of them. qemu-ga seems to support following APIs: $ ./qemu-ga --blacklist=? guest-sync-delimited guest-sync guest-ping guest-get-time guest-set-time guest-info guest-shutdown guest-file-open guest-file-close guest-file-read guest-file-write guest-file-seek guest-file-flush guest-fsfreeze-status guest-fsfreeze-freeze guest-fsfreeze-freeze-list guest-fsfreeze-thaw guest-fstrim guest-suspend-disk guest-suspend-ram guest-suspend-hybrid guest-network-get-interfaces guest-get-vcpus guest-set-vcpus guest-get-disks guest-get-fsinfo guest-set-user-password guest-get-memory-blocks guest-set-memory-blocks guest-get-memory-block-info guest-exec-status guest-exec guest-get-host-name guest-get-users guest-get-timezone guest-get-osinfo guest-get-devices guest-ssh-get-authorized-keys guest-ssh-add-authorized-keys guest-ssh-remove-authorized-keys Let's categorize them: Utility functions: guest-sync-delimited guest-sync guest-ping Functions which at least partially map to libvirt APIs: guest-shutdown guest-get-vcpus guest-set-vcpus Functions used internally but also useful externally: guest-fsfreeze-status guest-fsfreeze-freeze guest-fsfreeze-freeze-list guest-fsfreeze-thaw Functions which can be claimed to map to libvirt but don't really: guest-suspend-disk guest-suspend-ram guest-suspend-hybrid Functions which are not really relevant to libvirt but we expose wrappers: guest-get-time guest-set-time guest-fstrim guest-network-get-interfaces guest-get-disks guest-get-fsinfo guest-get-host-name guest-get-users guest-get-timezone guest-get-osinfo Functions which are not really relevant to libvirt, we expose wrappers and they are scary: guest-set-user-password Functions which are not really relevant to libvirt, we don't expose wrappers and they are scary: guest-file-open guest-file-close guest-file-read guest-file-write guest-file-seek guest-file-flush guest-exec-status guest-exec Other stats functions we might be asked for: guest-info guest-get-memory-blocks guest-set-memory-blocks guest-get-memory-block-info guest-get-devices And finally those we have here: guest-ssh-get-authorized-keys guest-ssh-add-authorized-keys guest-ssh-remove-authorized-keys So there's just a very tiny set of QGA APIs we really need. For the rest we are adding more-or-less direct wrappers to satisfy the access to the APIs. With more-and-more APIs added which really deal just with the guest state itself it's a bit weird to add libvirt APIs for them. I'd like us to prevent adding new APIs constantly especially those which deal just with the guest. Since libvirt actually can't ever rely on the state of the guest agent as it can be restarted at any time including the guest OS and thus we we have to internally treat it without any presumptions about state and re-probe everything and we really need just a tiny subset of the commands, we'd really benefint from just multiplexing access between libvirt and any other APP which can use the QGA QMP protocol directly. Thus I think we could arguably make virDomainQemuAgentCommand a supported API. Any guest state change which can be initiated via the GA needs to be supported also without that since the guest itself can trigger it. Saying that virDomainQemuAgentCommand is fully supported to be used would free us from having to add arbitrary unextendable APIs for every single guest agent API, but would still allow libvirt to use APIs we need. We just need to make 100% sure that it can't be abused as argument to to change status of virDomainQemuMonitorCommand.