Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 13:11:57 +0200, Eugenio Perez Martin wrote: > On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark wrote: > > > > On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote: > > > vDPA devices will be migratable soon. Since they are not migratable > > > before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking > > > for migration blockers, let it hardcoded in that case. > > > > > > Otherwise, ask qemu about the explicit blocker. > > > > > > Signed-off-by: Eugenio Pérez > > > --- > > > src/qemu/qemu_migration.c | 7 +-- > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > > > index 4224339f39..2f5c1d8121 100644 > > > --- a/src/qemu/qemu_migration.c > > > +++ b/src/qemu/qemu_migration.c > > > @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, > > > int pauseReason; > > > size_t i; > > > int r; > > > +bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, > > > + > > > QEMU_CAPS_MIGRATION_BLOCKED_REASONS); > > > > Wrong indentation of the second line (QEMU... should be aligned with > > priv). > > > > I'm fine with that indentation, but that will be a line with more than 80 > chars. No problem, it's not a hard requirement anymore in cases where it would make the result look worse. Jirka
Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 1:13 PM Peter Krempa wrote: > > On Wed, Jul 20, 2022 at 13:11:57 +0200, Eugenio Perez Martin wrote: > > On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark wrote: > > > > > > On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote: > > > > vDPA devices will be migratable soon. Since they are not migratable > > > > before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking > > > > for migration blockers, let it hardcoded in that case. > > > > > > > > Otherwise, ask qemu about the explicit blocker. > > > > > > > > Signed-off-by: Eugenio Pérez > > > > --- > > > > src/qemu/qemu_migration.c | 7 +-- > > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > > > > index 4224339f39..2f5c1d8121 100644 > > > > --- a/src/qemu/qemu_migration.c > > > > +++ b/src/qemu/qemu_migration.c > > > > @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, > > > > int pauseReason; > > > > size_t i; > > > > int r; > > > > +bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, > > > > + > > > > QEMU_CAPS_MIGRATION_BLOCKED_REASONS); > > > > > > Wrong indentation of the second line (QEMU... should be aligned with > > > priv). > > > > > > > I'm fine with that indentation, but that will be a line with more than 80 > > chars. > > The max 80 columns limitation is only a soft limit. We prefer way more > to keep alignment and readability of the code rather than stick to the > antiquated 80 columns limit. > Got it, I'll indent properly for the next version then. Thanks!
Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 13:11:57 +0200, Eugenio Perez Martin wrote: > On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark wrote: > > > > On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote: > > > vDPA devices will be migratable soon. Since they are not migratable > > > before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking > > > for migration blockers, let it hardcoded in that case. > > > > > > Otherwise, ask qemu about the explicit blocker. > > > > > > Signed-off-by: Eugenio Pérez > > > --- > > > src/qemu/qemu_migration.c | 7 +-- > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > > > index 4224339f39..2f5c1d8121 100644 > > > --- a/src/qemu/qemu_migration.c > > > +++ b/src/qemu/qemu_migration.c > > > @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, > > > int pauseReason; > > > size_t i; > > > int r; > > > +bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, > > > + > > > QEMU_CAPS_MIGRATION_BLOCKED_REASONS); > > > > Wrong indentation of the second line (QEMU... should be aligned with > > priv). > > > > I'm fine with that indentation, but that will be a line with more than 80 > chars. The max 80 columns limitation is only a soft limit. We prefer way more to keep alignment and readability of the code rather than stick to the antiquated 80 columns limit.
Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark wrote:
>
> On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote:
> > vDPA devices will be migratable soon. Since they are not migratable
> > before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking
> > for migration blockers, let it hardcoded in that case.
> >
> > Otherwise, ask qemu about the explicit blocker.
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > src/qemu/qemu_migration.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 4224339f39..2f5c1d8121 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
> > int pauseReason;
> > size_t i;
> > int r;
> > +bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps,
> > +
> > QEMU_CAPS_MIGRATION_BLOCKED_REASONS);
>
> Wrong indentation of the second line (QEMU... should be aligned with
> priv).
>
I'm fine with that indentation, but that will be a line with more than 80 chars.
> >
> > /* Ask qemu if it have a migration blocker */
> > -if (virQEMUCapsGet(priv->qemuCaps,
> > QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) {
> > +if (blockedReasonsCap) {
> > g_auto(GStrv) blockers = NULL;
> > r = qemuDomainGetMigrationBlockers(driver, vm, );
> > if (r != 0) {
> > @@ -1579,7 +1581,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
> > virDomainNetDef *net = vm->def->nets[i];
> > qemuSlirp *slirp;
> >
> > -if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
> > +
>
> Extra empty line.
>
I'll delete it in the next version.
> > +if (!blockedReasonsCap && net->type ==
> > VIR_DOMAIN_NET_TYPE_VDPA) {
>
> So possibly we could skip more checks in the function if migration
> blockers are supported by QEMU, but this is a good conservative
> approach. The other checks (if any) can be taken care of separately.
>
I did a fast search but I didn't see something super obvious to me.
Thanks!
> > virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > _("vDPA devices cannot be migrated"));
> > return false;
>
> Jirka
>
Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 12:37 PM Jiri Denemark wrote: > > On Wed, Jul 20, 2022 at 12:36:16 +0200, Jiri Denemark wrote: > > On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote: > > > vDPA devices will be migratable soon. Since they are not migratable > > > before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking > > > for migration blockers, let it hardcoded in that case. > > > > > > Otherwise, ask qemu about the explicit blocker. > > > > > > Signed-off-by: Eugenio Pérez > > > --- > > > src/qemu/qemu_migration.c | 7 +-- > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > Oh and I forgot to mention s/qemu_migrate/qemu_migration/ in the > subject. > I'll fix it in the next version. Thanks! > Jirka >
Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 12:36:16 +0200, Jiri Denemark wrote: > On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote: > > vDPA devices will be migratable soon. Since they are not migratable > > before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking > > for migration blockers, let it hardcoded in that case. > > > > Otherwise, ask qemu about the explicit blocker. > > > > Signed-off-by: Eugenio Pérez > > --- > > src/qemu/qemu_migration.c | 7 +-- > > 1 file changed, 5 insertions(+), 2 deletions(-) Oh and I forgot to mention s/qemu_migrate/qemu_migration/ in the subject. Jirka
Re: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 11:11:54 +0200, Eugenio Pérez wrote:
> vDPA devices will be migratable soon. Since they are not migratable
> before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking
> for migration blockers, let it hardcoded in that case.
>
> Otherwise, ask qemu about the explicit blocker.
>
> Signed-off-by: Eugenio Pérez
> ---
> src/qemu/qemu_migration.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 4224339f39..2f5c1d8121 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
> int pauseReason;
> size_t i;
> int r;
> +bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps,
> +
> QEMU_CAPS_MIGRATION_BLOCKED_REASONS);
Wrong indentation of the second line (QEMU... should be aligned with
priv).
>
> /* Ask qemu if it have a migration blocker */
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS))
> {
> +if (blockedReasonsCap) {
> g_auto(GStrv) blockers = NULL;
> r = qemuDomainGetMigrationBlockers(driver, vm, );
> if (r != 0) {
> @@ -1579,7 +1581,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
> virDomainNetDef *net = vm->def->nets[i];
> qemuSlirp *slirp;
>
> -if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
> +
Extra empty line.
> +if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA)
> {
So possibly we could skip more checks in the function if migration
blockers are supported by QEMU, but this is a good conservative
approach. The other checks (if any) can be taken care of separately.
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("vDPA devices cannot be migrated"));
> return false;
Jirka
[libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
vDPA devices will be migratable soon. Since they are not migratable
before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking
for migration blockers, let it hardcoded in that case.
Otherwise, ask qemu about the explicit blocker.
Signed-off-by: Eugenio Pérez
---
src/qemu/qemu_migration.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4224339f39..2f5c1d8121 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
int pauseReason;
size_t i;
int r;
+bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps,
+ QEMU_CAPS_MIGRATION_BLOCKED_REASONS);
/* Ask qemu if it have a migration blocker */
-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) {
+if (blockedReasonsCap) {
g_auto(GStrv) blockers = NULL;
r = qemuDomainGetMigrationBlockers(driver, vm, );
if (r != 0) {
@@ -1579,7 +1581,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
virDomainNetDef *net = vm->def->nets[i];
qemuSlirp *slirp;
-if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
+
+if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("vDPA devices cannot be migrated"));
return false;
--
2.31.1
