Kevin Wolf <kw...@redhat.com> writes: > Am 29.09.2015 um 22:20 hat John Snow geschrieben: >> If a migration is already in progress and somebody attempts >> to add a migration blocker, this should rightly fail. >> >> Add an errp parameter and a retcode return value to migrate_add_blocker. >> >> This is part one of two for a solution to prohibit e.g. block jobs >> from running concurrently with migration. >> >> Signed-off-by: John Snow <js...@redhat.com> [...] >> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c >> index cc76989..859e844 100644 >> --- a/hw/misc/ivshmem.c >> +++ b/hw/misc/ivshmem.c >> @@ -740,7 +740,10 @@ static int pci_ivshmem_init(PCIDevice *dev) >> if (s->role_val == IVSHMEM_PEER) { >> error_setg(&s->migration_blocker, >> "Migration is disabled when using feature 'peer mode' in >> device 'ivshmem'"); >> - migrate_add_blocker(s->migration_blocker); >> + if (migrate_add_blocker(s->migration_blocker, NULL) < 0) { >> + error_report("Unable to prohibit migration during ivshmem >> init"); >> + exit(1); > > Seriously? :-/ > > I see that the whole function does the same and has an int return value > only so that it can always return 0. So what you're doing is probably > right for this patch, but generally, the error handling in this init > function is horrible.
Horrible indeed. Marc-André's series to clean it up clocks in at 47 patches right now. Message-Id: <1443094669-4144-1-git-send-email-marcandre.lur...@redhat.com> http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg06302.html > The good thing is that you can't leak anything this way... > >> + } >> } >> >> pci_conf = dev->config; [...]