On Tue, Mar 11, 2025 at 08:08:02PM +0000, Daniel P. Berrangé wrote: > On Tue, Mar 11, 2025 at 03:57:35PM -0400, Peter Xu wrote: > > On Tue, Mar 11, 2025 at 03:33:23PM +0000, Daniel P. Berrangé wrote: > > > On Tue, Mar 11, 2025 at 11:20:50AM -0400, Peter Xu wrote: > > > > On Tue, Mar 11, 2025 at 08:13:16AM +0000, Daniel P. Berrangé wrote: > > > > > On Mon, Mar 10, 2025 at 04:03:26PM -0400, Peter Xu wrote: > > > > > > On Mon, Mar 10, 2025 at 07:48:16PM +0000, Daniel P. Berrangé wrote: > > > > > > > Given this is in public API, the data needs to remain reported > > > > > > > accurately > > > > > > > for the whole deprecation period. IOW, the patch to qiochannel > > > > > > > needs to > > > > > > > preserve this data too. > > > > > > > > > > > > :-( > > > > > > > > > > > > We could potentially mark MigrationStats to be experimental as a > > > > > > whole and > > > > > > declare that in deprecate.rst too, then after two releases, we can > > > > > > randomly > > > > > > add / remove fields as wish without always need to go through the > > > > > > deprecation process, am I right? > > > > > > > > > > IMHO that would be an abuse of the process and harmful to applications > > > > > and users consuming stats. > > > > > > > > Ah I just noticed that's the exact same one we included in > > > > query-migrate.. Then yes, the stable ABI is important here. > > > > > > > > So for this specific case, maybe we shouldn't have exposed it in QMP > > > > from > > > > the start. > > > > > > > > To me, it's a question on whether we could have something experimental > > > > and > > > > be exposed to QMP, where we don't need to guarantee a strict stable > > > > ABI, or > > > > a very loose ABI (e.g. we can guarantee the command exists, and with > > > > key-value string-integer pairs, nothing else). > > > > > > QMP has the ability to tag commands/fields, etc as experimental. > > > > > > libvirt will explicitly avoid consuming or exposing anything with > > > an experimental tag on it. > > > > > > > Maybe what we need is a new MigrationInfoOptional, to be embeded into > > > > MigrationInfo (or not), marked experimental. Then in the future > > > > whenever > > > > we want to add some new statistics, we could decide whether it should be > > > > part of stable ABI or not. > > > > > > That is not required - individual struct fields can be marked > > > experimental. > > > > Yes that'll work too. The important bit here is I think we should start to > > seriously evaluate which to expose to QAPI as stable API when we add stats > > into it. We used to not pay too much attention. > > > > With MigrationInfoOptional, we should suggest any new field to be added > > there by default, then whatever needs to be put out of experimental needs > > explicit justifications. Or we can also document any new migration field > > at least in the stats to be marked as experimental unless justified. > > > > > > > > The key question is what the intended usage of the fields/stats/etc > > > is to be. If you want it used by libvirt and mgmt apps it would need > > > to be formally supported. If it is just for adhoc QEMU developer > > > debugging and doesn't need libvirt / app support, then experimental > > > is fine. > > > > To my initial thoughts, I want Libvirt to fetch it. However I don't want > > Libvirt to parse it. > > > > For example, for things like "whether zerocopy send succeeded or not", or > > "how much time we spent on sending non-iterable device states", they're > > almost not consumable for users, but great for debuggings. It would be > > great if Libvirt could know their existance, fetch it (e.g. once after > > migration completes) then dump it to the logfile to help debugging and > > triaging QEMU issues. In that case parsing is not needed, the whole result > > can be attached to the log as a JSON blob. That releases the burden from > > the need to maintain compatibility that we don't really need and nobody > > cared (I bet it's the case here for zerocopy stats, but we got restricted > > by our promises even if it may ultimately benefit nobody..). > > We already log every single QMP command & response and event we deal > with, at INFO level, but by default our log files are only set to > capture WARN level, so this isn't visible without extra config steps > by the ademin > > Possibly we could think about dumping all migration stats to > /var/log/libvirt/qemu/$GUEST.log at migration completion
Yes it would be great to have it if it's trivial to get. It could be a last round of 'query-migrate' dumped only on src after migration is completed, right before src QEMU shuts down. Thanks, -- Peter Xu
