Currently, the migration code denies migration as long as the VM has a
vhost-vDPA device.
While it's true that vhost-vDPA devices cannot be migrated at the moment, there are plans to be able to migrate them soon.
Libvirt must treat it equal to vhost-kernel devices: Not all of them can
be migrated (the ones that do not expose the feature VHOST_F_LOG_ALL
cannot be migrated). So checks like this one should work for all vhost
devices equally.
A more accurate solution is to ask qemu if it has an active migration
blocker at that moment. Hoever, that require synchronization to avoid
qemu to add one between the query and the actual migration command.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
src/qemu/qemu_migration.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9c3fd41761..4ddf027c83 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1546,12 +1546,6 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
virDomainNetDef *net = vm->def->nets[i];
qemuSlirp *slirp;
- if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("vDPA devices cannot be migrated"));
- return false;
- }
-
slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
if (slirp && !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
--
2.31.1
On Fri, Jul 15, 2022 at 11:14:58 +0200, Eugenio Pérez wrote: > Currently, the migration code denies migration as long as the VM has a > vhost-vDPA device. > > While it's true that vhost-vDPA devices cannot be migrated at the moment, there are plans to be able to migrate them soon. > > Libvirt must treat it equal to vhost-kernel devices: Not all of them can > be migrated (the ones that do not expose the feature VHOST_F_LOG_ALL > cannot be migrated). So checks like this one should work for all vhost > devices equally. > > A more accurate solution is to ask qemu if it has an active migration > blocker at that moment. Hoever, that require synchronization to avoid > qemu to add one between the query and the actual migration command. What would cause QEMU to add such a blocker? I believe it wouldn't do it by itself and I hope even guest action would not cause migration blocker to be added. Which in ideal case would mean only a QMP command (such as hotplugging a non-migratable device) is the only way to add migration blocker. If this is true, than we're safe as libvirt does not allow such commands between qemuMigrationSrcIsAllowed and migration start. That said, is there a reason for not implementing the correct solution right away as a separate patch? Jirka
On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar@redhat.com> wrote:
>
> On Fri, Jul 15, 2022 at 11:14:58 +0200, Eugenio Pérez wrote:
> > Currently, the migration code denies migration as long as the VM has a
> > vhost-vDPA device.
> >
> > While it's true that vhost-vDPA devices cannot be migrated at the moment, there are plans to be able to migrate them soon.
> >
> > Libvirt must treat it equal to vhost-kernel devices: Not all of them can
> > be migrated (the ones that do not expose the feature VHOST_F_LOG_ALL
> > cannot be migrated). So checks like this one should work for all vhost
> > devices equally.
> >
> > A more accurate solution is to ask qemu if it has an active migration
> > blocker at that moment. Hoever, that require synchronization to avoid
> > qemu to add one between the query and the actual migration command.
>
> What would cause QEMU to add such a blocker? I believe it wouldn't do it
> by itself and I hope even guest action would not cause migration blocker
> to be added.
Qemu already adds a blocker in those cases. For example, in
hw/virtio/vhost.c:vhost_dev_init, you can find this conditional:
--
if (hdev->migration_blocker == NULL) {
if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
error_setg(&hdev->migration_blocker,
"Migration disabled: vhost lacks
VHOST_F_LOG_ALL feature.");
} else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_alloc_check()) {
error_setg(&hdev->migration_blocker,
"Migration disabled: failed to allocate shared memory");
}
}
if (hdev->migration_blocker != NULL) {
r = migrate_add_blocker(hdev->migration_blocker, errp);
...
}
--
So "migrate" command fails at monitor with that error message
> Which in ideal case would mean only a QMP command (such as
> hotplugging a non-migratable device) is the only way to add migration
> blocker. If this is true, than we're safe as libvirt does not allow such
> commands between qemuMigrationSrcIsAllowed and migration start.
>
Ok, that rules out a few bad use cases. I can do a fast lookup to
check if blockers can be added without the knowledge of libvirt.
> That said, is there a reason for not implementing the correct solution
> right away as a separate patch?
>
I was not sure if libvirt already had another way to check, for
example, if the vhost device didn't have VHOST_F_LOG_ALL feature. If
not, I think checking for blockers it's the right thing to do.
Thanks!
On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote: > On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar@redhat.com> wrote: > > Which in ideal case would mean only a QMP command (such as > > hotplugging a non-migratable device) is the only way to add migration > > blocker. If this is true, than we're safe as libvirt does not allow such > > commands between qemuMigrationSrcIsAllowed and migration start. > > > > Ok, that rules out a few bad use cases. I can do a fast lookup to > check if blockers can be added without the knowledge of libvirt. > > > That said, is there a reason for not implementing the correct solution > > right away as a separate patch? > > > > I was not sure if libvirt already had another way to check, for > example, if the vhost device didn't have VHOST_F_LOG_ALL feature. I'm not aware of such check, but even if it exists, checking for migration blockers looks like the right way of doing things anyway. Jirka
On 7/18/22 11:15 AM, Jiri Denemark wrote: > On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote: >> On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar@redhat.com> wrote: >>> Which in ideal case would mean only a QMP command (such as >>> hotplugging a non-migratable device) is the only way to add migration >>> blocker. If this is true, than we're safe as libvirt does not allow such >>> commands between qemuMigrationSrcIsAllowed and migration start. >>> >> >> Ok, that rules out a few bad use cases. I can do a fast lookup to >> check if blockers can be added without the knowledge of libvirt. >> >>> That said, is there a reason for not implementing the correct solution >>> right away as a separate patch? >>> >> >> I was not sure if libvirt already had another way to check, for >> example, if the vhost device didn't have VHOST_F_LOG_ALL feature. > > I'm not aware of such check, but even if it exists, checking for > migration blockers looks like the right way of doing things anyway. Actually that's been on my todo list for a long time - for any qemu that supports the QMP command that checks for migratability, we should be calling this command rather than checking against our own internal list (which is really just an "informed guess") of what can't be migrated. This way we'll always get the right answer (or at least what QEMU believes to be the right answer :-)). Fixing it this way will also mean that migration of VFIO devices will just "magically" start working once a migration-supporting driver is written for the device, and the correct vfio driver is bound to the device (this latter item is also on my list). So if you're up for making the patch to call the QMP command, I'd be happy to review it!
On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote: > > On 7/18/22 11:15 AM, Jiri Denemark wrote: > > On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote: > >> On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar@redhat.com> wrote: > >>> Which in ideal case would mean only a QMP command (such as > >>> hotplugging a non-migratable device) is the only way to add migration > >>> blocker. If this is true, than we're safe as libvirt does not allow such > >>> commands between qemuMigrationSrcIsAllowed and migration start. > >>> > >> > >> Ok, that rules out a few bad use cases. I can do a fast lookup to > >> check if blockers can be added without the knowledge of libvirt. > >> > >>> That said, is there a reason for not implementing the correct solution > >>> right away as a separate patch? > >>> > >> > >> I was not sure if libvirt already had another way to check, for > >> example, if the vhost device didn't have VHOST_F_LOG_ALL feature. > > > > I'm not aware of such check, but even if it exists, checking for > > migration blockers looks like the right way of doing things anyway. > > Actually that's been on my todo list for a long time - for any qemu that > supports the QMP command that checks for migratability, we should be > calling this command rather than checking against our own internal list > (which is really just an "informed guess") of what can't be migrated. > This way we'll always get the right answer (or at least what QEMU > believes to be the right answer :-)). Fixing it this way will also mean > that migration of VFIO devices will just "magically" start working once > a migration-supporting driver is written for the device, and the correct > vfio driver is bound to the device (this latter item is also on my list). > > So if you're up for making the patch to call the QMP command, I'd be > happy to review it! > Thanks! Actually I'd need some guidance first, I'm not very used to libvirt code. As I understand I should create a function in qemu_agent.h/c, a getter similar to qemuAgentGetFSInfo. How can I get a qemuAgent from qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there. For now it should be enough to delete vdpa hardcoded negation, and then other parts of libvirt can delete other hardcoded checks, isn't it? Thanks!
On 7/19/22 11:09 AM, Eugenio Perez Martin wrote: > On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote: >> >> On 7/18/22 11:15 AM, Jiri Denemark wrote: >>> On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote: >>>> On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar@redhat.com> wrote: >>>>> Which in ideal case would mean only a QMP command (such as >>>>> hotplugging a non-migratable device) is the only way to add migration >>>>> blocker. If this is true, than we're safe as libvirt does not allow such >>>>> commands between qemuMigrationSrcIsAllowed and migration start. >>>>> >>>> >>>> Ok, that rules out a few bad use cases. I can do a fast lookup to >>>> check if blockers can be added without the knowledge of libvirt. >>>> >>>>> That said, is there a reason for not implementing the correct solution >>>>> right away as a separate patch? >>>>> >>>> >>>> I was not sure if libvirt already had another way to check, for >>>> example, if the vhost device didn't have VHOST_F_LOG_ALL feature. >>> >>> I'm not aware of such check, but even if it exists, checking for >>> migration blockers looks like the right way of doing things anyway. >> >> Actually that's been on my todo list for a long time - for any qemu that >> supports the QMP command that checks for migratability, we should be >> calling this command rather than checking against our own internal list >> (which is really just an "informed guess") of what can't be migrated. >> This way we'll always get the right answer (or at least what QEMU >> believes to be the right answer :-)). Fixing it this way will also mean >> that migration of VFIO devices will just "magically" start working once >> a migration-supporting driver is written for the device, and the correct >> vfio driver is bound to the device (this latter item is also on my list). >> >> So if you're up for making the patch to call the QMP command, I'd be >> happy to review it! >> > > Thanks! Actually I'd need some guidance first, I'm not very used to > libvirt code. > > As I understand I should create a function in qemu_agent.h/c, a getter > similar to qemuAgentGetFSInfo. How can I get a qemuAgent from > qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there. qemu_agent.c is only for functions that require calling to the QEMU guest agent, which is a process running inside the guest. You just need to run a simple QMP command. There are some good examples of this in qemu_monitor_json.c > > For now it should be enough to delete vdpa hardcoded negation, and > then other parts of libvirt can delete other hardcoded checks, isn't > it? There's just a single function that checks for migratability (qemuMigrationSrcIsAllowed()). In theory *everything* in that function should be deprecated by just calling qemu to ask. In practice there may be / probably are things that qemu doesn't count as "can't migrate" that really should be counted that way. Certainly the VDPA and hostdev checks should be removable immediately though (although of course this should still be checked before pushing!) What I would do is this: 1) a patch that adds code to the qemu_capabilities to set a flag if the desired field in the "query-migrate" QMP command would be filled in by this qemu binary. 2) a patch that adds a function to qemu_monitor_json.c to call that command and return migratable/not. 3) a patch that adds a call to that function to qemuMigrationSrcIsAllowed(). 4) additional patches that remove specific hardcoded checks *only if the new field in query-migrate is available (as indicated by the new capabilities flag) and returned a definite yes/no (otherwise the checks still need to be done, to account for older qemu binaries that don't have the qmp command). I had thought there was a bugzilla somewhere that was tracking this for libvirt, but I can't find it.
On 7/19/22 12:01 PM, Laine Stump wrote: > On 7/19/22 11:09 AM, Eugenio Perez Martin wrote: >> On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote: >>> >>> On 7/18/22 11:15 AM, Jiri Denemark wrote: >>>> On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote: >>>>> On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark >>>>> <jdenemar@redhat.com> wrote: >>>>>> Which in ideal case would mean only a QMP command (such as >>>>>> hotplugging a non-migratable device) is the only way to add migration >>>>>> blocker. If this is true, than we're safe as libvirt does not >>>>>> allow such >>>>>> commands between qemuMigrationSrcIsAllowed and migration start. >>>>>> >>>>> >>>>> Ok, that rules out a few bad use cases. I can do a fast lookup to >>>>> check if blockers can be added without the knowledge of libvirt. >>>>> >>>>>> That said, is there a reason for not implementing the correct >>>>>> solution >>>>>> right away as a separate patch? >>>>>> >>>>> >>>>> I was not sure if libvirt already had another way to check, for >>>>> example, if the vhost device didn't have VHOST_F_LOG_ALL feature. >>>> >>>> I'm not aware of such check, but even if it exists, checking for >>>> migration blockers looks like the right way of doing things anyway. >>> >>> Actually that's been on my todo list for a long time - for any qemu that >>> supports the QMP command that checks for migratability, we should be >>> calling this command rather than checking against our own internal list >>> (which is really just an "informed guess") of what can't be migrated. >>> This way we'll always get the right answer (or at least what QEMU >>> believes to be the right answer :-)). Fixing it this way will also mean >>> that migration of VFIO devices will just "magically" start working once >>> a migration-supporting driver is written for the device, and the correct >>> vfio driver is bound to the device (this latter item is also on my >>> list). >>> >>> So if you're up for making the patch to call the QMP command, I'd be >>> happy to review it! >>> >> >> Thanks! Actually I'd need some guidance first, I'm not very used to >> libvirt code. >> >> As I understand I should create a function in qemu_agent.h/c, a getter >> similar to qemuAgentGetFSInfo. How can I get a qemuAgent from >> qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there. > > qemu_agent.c is only for functions that require calling to the QEMU > guest agent, which is a process running inside the guest. You just need > to run a simple QMP command. There are some good examples of this in > qemu_monitor_json.c > >> >> For now it should be enough to delete vdpa hardcoded negation, and >> then other parts of libvirt can delete other hardcoded checks, isn't >> it? > > There's just a single function that checks for migratability > (qemuMigrationSrcIsAllowed()). In theory *everything* in that function > should be deprecated by just calling qemu to ask. In practice there may > be / probably are things that qemu doesn't count as "can't migrate" that > really should be counted that way. Certainly the VDPA and hostdev checks > should be removable immediately though (although of course this should > still be checked before pushing!) > > > What I would do is this: > > 1) a patch that adds code to the qemu_capabilities to set a flag if the > desired field in the "query-migrate" QMP command would be filled in by > this qemu binary. Just to permanently document live discussions from IRC: jjongsma pointed us to a patch he wrote a year ago (but never pushed upstream) that implements (1): https://gitlab.com/jjongsma/libvirt/-/commit/4003b7047058a17465083178d6c0a0f62c900c74 > > 2) a patch that adds a function to qemu_monitor_json.c to call that > command and return migratable/not. Thinking about this more, I guess a function that returns the full text of "blocked-reason" would be more useful (that way it could be easily logged). > 3) a patch that adds a call to that function to > qemuMigrationSrcIsAllowed(). > > 4) additional patches that remove specific hardcoded checks *only if the > new field in query-migrate is available (as indicated by the new > capabilities flag) and returned a definite yes/no (otherwise the checks > still need to be done, to account for older qemu binaries that don't > have the qmp command). > > I had thought there was a bugzilla somewhere that was tracking this for > libvirt, but I can't find it. >
On Tue, Jul 19, 2022 at 6:43 PM Laine Stump <laine@laine.org> wrote: > > On 7/19/22 12:01 PM, Laine Stump wrote: > > On 7/19/22 11:09 AM, Eugenio Perez Martin wrote: > >> On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote: > >>> > >>> On 7/18/22 11:15 AM, Jiri Denemark wrote: > >>>> On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote: > >>>>> On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark > >>>>> <jdenemar@redhat.com> wrote: > >>>>>> Which in ideal case would mean only a QMP command (such as > >>>>>> hotplugging a non-migratable device) is the only way to add migration > >>>>>> blocker. If this is true, than we're safe as libvirt does not > >>>>>> allow such > >>>>>> commands between qemuMigrationSrcIsAllowed and migration start. > >>>>>> > >>>>> > >>>>> Ok, that rules out a few bad use cases. I can do a fast lookup to > >>>>> check if blockers can be added without the knowledge of libvirt. > >>>>> > >>>>>> That said, is there a reason for not implementing the correct > >>>>>> solution > >>>>>> right away as a separate patch? > >>>>>> > >>>>> > >>>>> I was not sure if libvirt already had another way to check, for > >>>>> example, if the vhost device didn't have VHOST_F_LOG_ALL feature. > >>>> > >>>> I'm not aware of such check, but even if it exists, checking for > >>>> migration blockers looks like the right way of doing things anyway. > >>> > >>> Actually that's been on my todo list for a long time - for any qemu that > >>> supports the QMP command that checks for migratability, we should be > >>> calling this command rather than checking against our own internal list > >>> (which is really just an "informed guess") of what can't be migrated. > >>> This way we'll always get the right answer (or at least what QEMU > >>> believes to be the right answer :-)). Fixing it this way will also mean > >>> that migration of VFIO devices will just "magically" start working once > >>> a migration-supporting driver is written for the device, and the correct > >>> vfio driver is bound to the device (this latter item is also on my > >>> list). > >>> > >>> So if you're up for making the patch to call the QMP command, I'd be > >>> happy to review it! > >>> > >> > >> Thanks! Actually I'd need some guidance first, I'm not very used to > >> libvirt code. > >> > >> As I understand I should create a function in qemu_agent.h/c, a getter > >> similar to qemuAgentGetFSInfo. How can I get a qemuAgent from > >> qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there. > > > > qemu_agent.c is only for functions that require calling to the QEMU > > guest agent, which is a process running inside the guest. You just need > > to run a simple QMP command. There are some good examples of this in > > qemu_monitor_json.c > > > >> > >> For now it should be enough to delete vdpa hardcoded negation, and > >> then other parts of libvirt can delete other hardcoded checks, isn't > >> it? > > > > There's just a single function that checks for migratability > > (qemuMigrationSrcIsAllowed()). In theory *everything* in that function > > should be deprecated by just calling qemu to ask. In practice there may > > be / probably are things that qemu doesn't count as "can't migrate" that > > really should be counted that way. Certainly the VDPA and hostdev checks > > should be removable immediately though (although of course this should > > still be checked before pushing!) > > > > > > What I would do is this: > > > > 1) a patch that adds code to the qemu_capabilities to set a flag if the > > desired field in the "query-migrate" QMP command would be filled in by > > this qemu binary. > > Just to permanently document live discussions from IRC: > > jjongsma pointed us to a patch he wrote a year ago (but never pushed > upstream) that implements (1): > > https://gitlab.com/jjongsma/libvirt/-/commit/4003b7047058a17465083178d6c0a0f62c900c74 > Thanks! > > > > 2) a patch that adds a function to qemu_monitor_json.c to call that > > command and return migratable/not. > > Thinking about this more, I guess a function that returns the full text > of "blocked-reason" would be more useful (that way it could be easily > logged). > I'm actually returned all the array in the form of `char ***` > > 3) a patch that adds a call to that function to > > qemuMigrationSrcIsAllowed(). > > What I cannot find an example of is how to get the qemuMonitor from the virQEMUDriver that qemuMigrationSrcIsAllowed has as the parameter. > > 4) additional patches that remove specific hardcoded checks *only if the > > new field in query-migrate is available (as indicated by the new > > capabilities flag) and returned a definite yes/no (otherwise the checks > > still need to be done, to account for older qemu binaries that don't > > have the qmp command). > > Yes, I agree. Thanks! > > I had thought there was a bugzilla somewhere that was tracking this for > > libvirt, but I can't find it. > > >
On 7/19/22 2:38 PM, Eugenio Perez Martin wrote:
> On Tue, Jul 19, 2022 at 6:43 PM Laine Stump <laine@laine.org> wrote:
>>
>> On 7/19/22 12:01 PM, Laine Stump wrote:
>>> On 7/19/22 11:09 AM, Eugenio Perez Martin wrote:
>>>> On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote:
>>>>>
>>>>> On 7/18/22 11:15 AM, Jiri Denemark wrote:
>>>>>> On Mon, Jul 18, 2022 at 10:40:56 +0200, Eugenio Perez Martin wrote:
>>>>>>> On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark
>>>>>>> <jdenemar@redhat.com> wrote:
>>>>>>>> Which in ideal case would mean only a QMP command (such as
>>>>>>>> hotplugging a non-migratable device) is the only way to add migration
>>>>>>>> blocker. If this is true, than we're safe as libvirt does not
>>>>>>>> allow such
>>>>>>>> commands between qemuMigrationSrcIsAllowed and migration start.
>>>>>>>>
>>>>>>>
>>>>>>> Ok, that rules out a few bad use cases. I can do a fast lookup to
>>>>>>> check if blockers can be added without the knowledge of libvirt.
>>>>>>>
>>>>>>>> That said, is there a reason for not implementing the correct
>>>>>>>> solution
>>>>>>>> right away as a separate patch?
>>>>>>>>
>>>>>>>
>>>>>>> I was not sure if libvirt already had another way to check, for
>>>>>>> example, if the vhost device didn't have VHOST_F_LOG_ALL feature.
>>>>>>
>>>>>> I'm not aware of such check, but even if it exists, checking for
>>>>>> migration blockers looks like the right way of doing things anyway.
>>>>>
>>>>> Actually that's been on my todo list for a long time - for any qemu that
>>>>> supports the QMP command that checks for migratability, we should be
>>>>> calling this command rather than checking against our own internal list
>>>>> (which is really just an "informed guess") of what can't be migrated.
>>>>> This way we'll always get the right answer (or at least what QEMU
>>>>> believes to be the right answer :-)). Fixing it this way will also mean
>>>>> that migration of VFIO devices will just "magically" start working once
>>>>> a migration-supporting driver is written for the device, and the correct
>>>>> vfio driver is bound to the device (this latter item is also on my
>>>>> list).
>>>>>
>>>>> So if you're up for making the patch to call the QMP command, I'd be
>>>>> happy to review it!
>>>>>
>>>>
>>>> Thanks! Actually I'd need some guidance first, I'm not very used to
>>>> libvirt code.
>>>>
>>>> As I understand I should create a function in qemu_agent.h/c, a getter
>>>> similar to qemuAgentGetFSInfo. How can I get a qemuAgent from
>>>> qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there.
>>>
>>> qemu_agent.c is only for functions that require calling to the QEMU
>>> guest agent, which is a process running inside the guest. You just need
>>> to run a simple QMP command. There are some good examples of this in
>>> qemu_monitor_json.c
>>>
>>>>
>>>> For now it should be enough to delete vdpa hardcoded negation, and
>>>> then other parts of libvirt can delete other hardcoded checks, isn't
>>>> it?
>>>
>>> There's just a single function that checks for migratability
>>> (qemuMigrationSrcIsAllowed()). In theory *everything* in that function
>>> should be deprecated by just calling qemu to ask. In practice there may
>>> be / probably are things that qemu doesn't count as "can't migrate" that
>>> really should be counted that way. Certainly the VDPA and hostdev checks
>>> should be removable immediately though (although of course this should
>>> still be checked before pushing!)
>>>
>>>
>>> What I would do is this:
>>>
>>> 1) a patch that adds code to the qemu_capabilities to set a flag if the
>>> desired field in the "query-migrate" QMP command would be filled in by
>>> this qemu binary.
>>
>> Just to permanently document live discussions from IRC:
>>
>> jjongsma pointed us to a patch he wrote a year ago (but never pushed
>> upstream) that implements (1):
>>
>> https://gitlab.com/jjongsma/libvirt/-/commit/4003b7047058a17465083178d6c0a0f62c900c74
>>
>
> Thanks!
>
>>>
>>> 2) a patch that adds a function to qemu_monitor_json.c to call that
>>> command and return migratable/not.
>>
>> Thinking about this more, I guess a function that returns the full text
>> of "blocked-reason" would be more useful (that way it could be easily
>> logged).
>>
>
> I'm actually returned all the array in the form of `char ***`
>
>>> 3) a patch that adds a call to that function to
>>> qemuMigrationSrcIsAllowed().
>>>
>
> What I cannot find an example of is how to get the qemuMonitor from
> the virQEMUDriver that qemuMigrationSrcIsAllowed has as the parameter.
Hopefully I'm not leading you down a false path, but it looks like the
call chain of:
qemuDomainChangeMemoryRequestedSize()
qemuMonitorChangeMemoryRequestedSize()
qemuMonitorJSONChangeMemoryRequestedSize()
contains all the bits you need, including the toplevel function that
calls qemuDomainObj(Enter|Exit)Monitor() and grabs the qemuMonitor
object from the domainObj's privateData before calling down to a wrapper
function in qemu_monitor.c (that I think was originally there because
there could be either an old-style shell-like command or a QMP command
to do the same thing), and then from there down to the QMP/JSON function
in qemu_monitor_json.c that actually calls the monitor.
>
>>> 4) additional patches that remove specific hardcoded checks *only if the
>>> new field in query-migrate is available (as indicated by the new
>>> capabilities flag) and returned a definite yes/no (otherwise the checks
>>> still need to be done, to account for older qemu binaries that don't
>>> have the qmp command).
>>>
>
> Yes, I agree.
>
> Thanks!
>
>>> I had thought there was a bugzilla somewhere that was tracking this for
>>> libvirt, but I can't find it.
>>>
>>
>
On Tue, Jul 19, 2022 at 17:09:29 +0200, Eugenio Perez Martin wrote: > On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote: > > Actually that's been on my todo list for a long time - for any qemu that > > supports the QMP command that checks for migratability, we should be > > calling this command rather than checking against our own internal list > > (which is really just an "informed guess") of what can't be migrated. > > This way we'll always get the right answer (or at least what QEMU > > believes to be the right answer :-)). Fixing it this way will also mean > > that migration of VFIO devices will just "magically" start working once > > a migration-supporting driver is written for the device, and the correct > > vfio driver is bound to the device (this latter item is also on my list). > > > > So if you're up for making the patch to call the QMP command, I'd be > > happy to review it! > > > > Thanks! Actually I'd need some guidance first, I'm not very used to > libvirt code. > > As I understand I should create a function in qemu_agent.h/c, a getter > similar to qemuAgentGetFSInfo. How can I get a qemuAgent from > qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there. I hope we don't need to ask the guest agent to get migration blockers as that would be pretty much a show stopper for us. What is the exact way of getting the info from QEMU? Jirka
On Tue, Jul 19, 2022 at 5:24 PM Jiri Denemark <jdenemar@redhat.com> wrote:
>
> On Tue, Jul 19, 2022 at 17:09:29 +0200, Eugenio Perez Martin wrote:
> > On Tue, Jul 19, 2022 at 4:02 PM Laine Stump <laine@laine.org> wrote:
> > > Actually that's been on my todo list for a long time - for any qemu that
> > > supports the QMP command that checks for migratability, we should be
> > > calling this command rather than checking against our own internal list
> > > (which is really just an "informed guess") of what can't be migrated.
> > > This way we'll always get the right answer (or at least what QEMU
> > > believes to be the right answer :-)). Fixing it this way will also mean
> > > that migration of VFIO devices will just "magically" start working once
> > > a migration-supporting driver is written for the device, and the correct
> > > vfio driver is bound to the device (this latter item is also on my list).
> > >
> > > So if you're up for making the patch to call the QMP command, I'd be
> > > happy to review it!
> > >
> >
> > Thanks! Actually I'd need some guidance first, I'm not very used to
> > libvirt code.
> >
> > As I understand I should create a function in qemu_agent.h/c, a getter
> > similar to qemuAgentGetFSInfo. How can I get a qemuAgent from
> > qemuMigrationSrcIsAllowed? I only have a virQEMUDriver there.
>
> I hope we don't need to ask the guest agent to get migration blockers as
> that would be pretty much a show stopper for us. What is the exact way
> of getting the info from QEMU?
>
Sorry, I confused "agent" with the QMP here.
This is an example response with a migration blocker:
<- '{ "execute": "query-migrate" }'
-> {"return": {"blocked-reasons": ["Migration disabled: vhost lacks
VHOST_F_LOG_ALL feature."]}}
There is no member "blocked-reasons" in case no blocker is registered in qemu.
> Jirka
>
© 2016 - 2024 Red Hat, Inc.