On 8/3/21 3:59 PM, Laine Stump wrote: > On 7/29/21 5:42 AM, Michal Prívozník wrote: >> On 7/28/21 6:17 PM, Kristina Hanicova wrote: >>> When we try to migrate vm, we check if it contains only devices >>> that are able to migrate. If a hostdev device is not able to >>> migrate we raise an error with <hostdev/>, but it can actually be >>> <interface/>, so we need to check if hostdev device was created >>> by us from interface and show the right error message. >>> >>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942315 >>> >>> Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx> >>> --- >>> src/qemu/qemu_migration.c | 12 +++++++++--- >>> 1 file changed, 9 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c >>> index 4d651aeb1a..34eee9c8b6 100644 >>> --- a/src/qemu/qemu_migration.c >>> +++ b/src/qemu/qemu_migration.c >>> @@ -1272,9 +1272,15 @@ qemuMigrationSrcIsAllowedHostdev(const >>> virDomainDef *def) >>> } >>> /* all other PCI hostdevs can't be migrated */ >>> - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, >>> - _("cannot migrate a domain with >>> <hostdev mode='subsystem' type='%s'>"), >>> - >>> virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type)); >>> + if (hostdev->parentnet) { >>> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, >>> + _("cannot migrate a domain with >>> <interface type='%s'>"), >>> + >>> virDomainNetTypeToString(hostdev->parentnet->type)); >> >> Small nit, I wonder whether we should report actual type here. Looking >> into virDomainNetDefFormat() it looks like for a running VM we do report >> actual type (unless inactive or migratable XML was requested). Thus I >> think the error message should follow that logic. Otherwise we might >> report "cannot migrate a domain with interface type=network" while in >> fact in 'virsh dumpxml' there is just interface type='hostdev' (the >> type='network' is in inactive XML). >> >> Laine, what's your opinion? > > > Well.... neither is ideal :-/. If the log message says "interface > type='network'" then that could mislead the user into thinking that no > type='network' interfaces would be supported. But if the log message > says "interface type='hostdev'" then the user will look in their XML, > not find any type='hostdev', and then be confused about what the message > is referencing. They would find type='hostdev' in the live XML and type='network' in inactive one. > > Ideally the error message should make it simple to find the exact > element causing the problem while also being precise in explaining what > is problematic. Since neither of the choices here satisfy both, we just > have to pick the 'least bad' option. I do think in this case that the > least bad option is to display tha actualtype as you suggest (which, > BTW, in this case will always be "hostdev"). > Agreed. Fixed and pushed. Michal
