| Message ID | 20210125132238.179972-1-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] s390x/cpu_model: disallow unpack for --only-migratable | expand |
Patchew URL: https://patchew.org/QEMU/20210125132238.179972-1-borntraeger@de.ibm.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210125132238.179972-1-borntraeger@de.ibm.com Subject: [PATCH v2] s390x/cpu_model: disallow unpack for --only-migratable === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20210125125312.138491-1-borntraeger@de.ibm.com -> patchew/20210125125312.138491-1-borntraeger@de.ibm.com * [new tag] patchew/20210125132238.179972-1-borntraeger@de.ibm.com -> patchew/20210125132238.179972-1-borntraeger@de.ibm.com Switched to a new branch 'test' 9d55ad2 s390x/cpu_model: disallow unpack for --only-migratable === OUTPUT BEGIN === ERROR: code indent should never use tabs #37: FILE: target/s390x/cpu_models.c:886: +^Ireturn;$ total: 1 errors, 0 warnings, 21 lines checked Commit 9d55ad23e018 (s390x/cpu_model: disallow unpack for --only-migratable) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210125132238.179972-1-borntraeger@de.ibm.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 25.01.21 14:22, Christian Borntraeger wrote: > secure execution (aka protected virtualization) guests cannot be > migrated at the moment. Disallow the unpack facility if the user > specifies --only-migratable. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > v1->v2: > - add missing return > - protect check with CONFIG_USER_ONLY for building non softmmu binaries > > target/s390x/cpu_models.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > index 35179f9dc7ba..e844a4007210 100644 > --- a/target/s390x/cpu_models.c > +++ b/target/s390x/cpu_models.c > @@ -26,6 +26,7 @@ > #include "qapi/qmp/qdict.h" > #ifndef CONFIG_USER_ONLY > #include "sysemu/arch_init.h" > +#include "sysemu/sysemu.h" > #include "hw/pci/pci.h" > #endif > #include "qapi/qapi-commands-machine-target.h" > @@ -878,6 +879,14 @@ static void check_compatibility(const S390CPUModel *max_model, > return; > } > > +#ifndef CONFIG_USER_ONLY > + if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) { > + error_setg(errp, "The unpack facility is not compatible with " > + "the --only-migratable option"); > + return; > + } > +#endif > + > /* detect the missing features to properly report them */ > bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX); > if (bitmap_empty(missing, S390_FEAT_MAX)) { > BTW, I wonder if the right place for this would be apply_cpu_model(). Anyhow Reviewed-by: David Hildenbrand <david@redhat.com>
On Mon, 25 Jan 2021 14:22:38 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > secure execution (aka protected virtualization) guests cannot be > migrated at the moment. Disallow the unpack facility if the user > specifies --only-migratable. Maybe make the explanation a bit more verbose? "Secure execution (aka protected virtualization) guests cannot be migrated at the moment. If the unpack facility is provided in the cpu model, a guest may choose to transition to secure mode, making the guest unmigratable at that point in time. If the machine was explicitly started with --only-migratable, we would get a failure only when the guest actually tries to transition; instead, explicitly disallow the unpack facility if --only-migratable was specified to avoid late surprises." > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > v1->v2: > - add missing return > - protect check with CONFIG_USER_ONLY for building non softmmu binaries > > target/s390x/cpu_models.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > index 35179f9dc7ba..e844a4007210 100644 > --- a/target/s390x/cpu_models.c > +++ b/target/s390x/cpu_models.c > @@ -26,6 +26,7 @@ > #include "qapi/qmp/qdict.h" > #ifndef CONFIG_USER_ONLY > #include "sysemu/arch_init.h" > +#include "sysemu/sysemu.h" > #include "hw/pci/pci.h" > #endif > #include "qapi/qapi-commands-machine-target.h" > @@ -878,6 +879,14 @@ static void check_compatibility(const S390CPUModel *max_model, > return; > } > > +#ifndef CONFIG_USER_ONLY > + if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) { > + error_setg(errp, "The unpack facility is not compatible with " > + "the --only-migratable option"); Might be a bit surprising if the host model had been specified... is there a way to add a hint how to get rid of the unpack bit? > + return; > + } > +#endif > + > /* detect the missing features to properly report them */ > bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX); > if (bitmap_empty(missing, S390_FEAT_MAX)) {
On 25.01.21 14:38, Cornelia Huck wrote: > On Mon, 25 Jan 2021 14:22:38 +0100 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> secure execution (aka protected virtualization) guests cannot be >> migrated at the moment. Disallow the unpack facility if the user >> specifies --only-migratable. > > Maybe make the explanation a bit more verbose? > > "Secure execution (aka protected virtualization) guests cannot be > migrated at the moment. If the unpack facility is provided in the cpu > model, a guest may choose to transition to secure mode, making the > guest unmigratable at that point in time. If the machine was explicitly > started with --only-migratable, we would get a failure only when the > guest actually tries to transition; instead, explicitly disallow the > unpack facility if --only-migratable was specified to avoid late > surprises." > >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> v1->v2: >> - add missing return >> - protect check with CONFIG_USER_ONLY for building non softmmu binaries >> >> target/s390x/cpu_models.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c >> index 35179f9dc7ba..e844a4007210 100644 >> --- a/target/s390x/cpu_models.c >> +++ b/target/s390x/cpu_models.c >> @@ -26,6 +26,7 @@ >> #include "qapi/qmp/qdict.h" >> #ifndef CONFIG_USER_ONLY >> #include "sysemu/arch_init.h" >> +#include "sysemu/sysemu.h" >> #include "hw/pci/pci.h" >> #endif >> #include "qapi/qapi-commands-machine-target.h" >> @@ -878,6 +879,14 @@ static void check_compatibility(const S390CPUModel *max_model, >> return; >> } >> >> +#ifndef CONFIG_USER_ONLY >> + if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) { >> + error_setg(errp, "The unpack facility is not compatible with " >> + "the --only-migratable option"); > > Might be a bit surprising if the host model had been specified... is > there a way to add a hint how to get rid of the unpack bit? I can try to make the error message a bit more verbose. > >> + return; >> + } >> +#endif >> + >> /* detect the missing features to properly report them */ >> bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX); >> if (bitmap_empty(missing, S390_FEAT_MAX)) { >
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 35179f9dc7ba..e844a4007210 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -26,6 +26,7 @@ #include "qapi/qmp/qdict.h" #ifndef CONFIG_USER_ONLY #include "sysemu/arch_init.h" +#include "sysemu/sysemu.h" #include "hw/pci/pci.h" #endif #include "qapi/qapi-commands-machine-target.h" @@ -878,6 +879,14 @@ static void check_compatibility(const S390CPUModel *max_model, return; } +#ifndef CONFIG_USER_ONLY + if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) { + error_setg(errp, "The unpack facility is not compatible with " + "the --only-migratable option"); + return; + } +#endif + /* detect the missing features to properly report them */ bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX); if (bitmap_empty(missing, S390_FEAT_MAX)) {
secure execution (aka protected virtualization) guests cannot be migrated at the moment. Disallow the unpack facility if the user specifies --only-migratable. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- v1->v2: - add missing return - protect check with CONFIG_USER_ONLY for building non softmmu binaries target/s390x/cpu_models.c | 9 +++++++++ 1 file changed, 9 insertions(+)