diff mbox

[v12,28/28] tests/qmp-test: blacklist sev specific qmp commands

Message ID 20180308124901.83533-29-brijesh.singh@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Brijesh Singh March 8, 2018, 12:49 p.m. UTC
Blacklist the following commands to fix the 'make check' failure.

query-sev-launch-measure: it returns meaninful data only when we launch
SEV guest otherwise the command returns an error.

query-sev: it return an error when SEV is not available on host (e.g non
X86 platform or KVM is disabled at the build time)

query-sev-capabilities: it returns an error when SEV feature is not
available on host machine.

Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 tests/qmp-test.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Daniel P. Berrangé March 8, 2018, 5:08 p.m. UTC | #1
On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
> Blacklist the following commands to fix the 'make check' failure.
> 
> query-sev-launch-measure: it returns meaninful data only when we launch
> SEV guest otherwise the command returns an error.
> 
> query-sev: it return an error when SEV is not available on host (e.g non
> X86 platform or KVM is disabled at the build time)
> 
> query-sev-capabilities: it returns an error when SEV feature is not
> available on host machine.

We generally expect 'make check' to succeed on every single patch
in a series, so that 'git bisect' doesn't break.

So you should add each command to the blacklist in the same commit
that introduced the failure in the first place.

> 
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  tests/qmp-test.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index 22445d9ec258..7470c6b754bc 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -204,6 +204,11 @@ static bool query_is_blacklisted(const char *cmd)
>          "query-gic-capabilities", /* arm */
>          /* Success depends on target-specific build configuration: */
>          "query-pci",              /* CONFIG_PCI */
> +        /* Success depends on launching SEV guest */
> +        "query-sev-launch-measure",
> +        /* Success depends on Host or Hypervisor SEV support */
> +        "query-sev",
> +        "query-sev-capabilities",
>          NULL
>      };
>      int i;
> -- 
> 2.14.3
> 

Regards,
Daniel
Brijesh Singh March 8, 2018, 8:18 p.m. UTC | #2
On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
>> Blacklist the following commands to fix the 'make check' failure.
>>
>> query-sev-launch-measure: it returns meaninful data only when we launch
>> SEV guest otherwise the command returns an error.
>>
>> query-sev: it return an error when SEV is not available on host (e.g non
>> X86 platform or KVM is disabled at the build time)
>>
>> query-sev-capabilities: it returns an error when SEV feature is not
>> available on host machine.
> We generally expect 'make check' to succeed on every single patch
> in a series, so that 'git bisect' doesn't break.
>
> So you should add each command to the blacklist in the same commit
> that introduced the failure in the first place.


Sure, I can quickly send the updated patch series to address your this
concern, but before spamming everyone's inbox I was wondering if I can
get some indication whether this series will make into 2.12 merge.

Paolo, Eduardo and Richard,

Most of the changes are in x86 directory hence any thought if you are
considering this series for 2.12 ? I have been testing the series with
and without SEV support and so far have not ran into any issue. if you
are not planning to pull this series in 2.12 then I will wait a bit
longer to get more feedback before sending the updates to address
Daniel's comment. thanks


 
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>  tests/qmp-test.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>> index 22445d9ec258..7470c6b754bc 100644
>> --- a/tests/qmp-test.c
>> +++ b/tests/qmp-test.c
>> @@ -204,6 +204,11 @@ static bool query_is_blacklisted(const char *cmd)
>>          "query-gic-capabilities", /* arm */
>>          /* Success depends on target-specific build configuration: */
>>          "query-pci",              /* CONFIG_PCI */
>> +        /* Success depends on launching SEV guest */
>> +        "query-sev-launch-measure",
>> +        /* Success depends on Host or Hypervisor SEV support */
>> +        "query-sev",
>> +        "query-sev-capabilities",
>>          NULL
>>      };
>>      int i;
>> -- 
>> 2.14.3
>>
> Regards,
> Daniel
Eduardo Habkost March 8, 2018, 9:45 p.m. UTC | #3
On Thu, Mar 08, 2018 at 02:18:55PM -0600, Brijesh Singh wrote:
> 
> 
> On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
> > On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
> >> Blacklist the following commands to fix the 'make check' failure.
> >>
> >> query-sev-launch-measure: it returns meaninful data only when we launch
> >> SEV guest otherwise the command returns an error.
> >>
> >> query-sev: it return an error when SEV is not available on host (e.g non
> >> X86 platform or KVM is disabled at the build time)
> >>
> >> query-sev-capabilities: it returns an error when SEV feature is not
> >> available on host machine.
> > We generally expect 'make check' to succeed on every single patch
> > in a series, so that 'git bisect' doesn't break.
> >
> > So you should add each command to the blacklist in the same commit
> > that introduced the failure in the first place.
> 
> 
> Sure, I can quickly send the updated patch series to address your this
> concern, but before spamming everyone's inbox I was wondering if I can
> get some indication whether this series will make into 2.12 merge.
> 
> Paolo, Eduardo and Richard,
> 
> Most of the changes are in x86 directory hence any thought if you are
> considering this series for 2.12 ? I have been testing the series with
> and without SEV support and so far have not ran into any issue. if you
> are not planning to pull this series in 2.12 then I will wait a bit
> longer to get more feedback before sending the updates to address
> Daniel's comment. thanks

Trying to merge it before 2.12 soft freeze (next Tuesday) still
looks like a reasonable goal to me.  What do others think?
Daniel P. Berrangé March 8, 2018, 11:22 p.m. UTC | #4
On Thu, Mar 08, 2018 at 06:45:04PM -0300, Eduardo Habkost wrote:
> On Thu, Mar 08, 2018 at 02:18:55PM -0600, Brijesh Singh wrote:
> > 
> > 
> > On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
> > > On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
> > >> Blacklist the following commands to fix the 'make check' failure.
> > >>
> > >> query-sev-launch-measure: it returns meaninful data only when we launch
> > >> SEV guest otherwise the command returns an error.
> > >>
> > >> query-sev: it return an error when SEV is not available on host (e.g non
> > >> X86 platform or KVM is disabled at the build time)
> > >>
> > >> query-sev-capabilities: it returns an error when SEV feature is not
> > >> available on host machine.
> > > We generally expect 'make check' to succeed on every single patch
> > > in a series, so that 'git bisect' doesn't break.
> > >
> > > So you should add each command to the blacklist in the same commit
> > > that introduced the failure in the first place.
> > 
> > 
> > Sure, I can quickly send the updated patch series to address your this
> > concern, but before spamming everyone's inbox I was wondering if I can
> > get some indication whether this series will make into 2.12 merge.
> > 
> > Paolo, Eduardo and Richard,
> > 
> > Most of the changes are in x86 directory hence any thought if you are
> > considering this series for 2.12 ? I have been testing the series with
> > and without SEV support and so far have not ran into any issue. if you
> > are not planning to pull this series in 2.12 then I will wait a bit
> > longer to get more feedback before sending the updates to address
> > Daniel's comment. thanks
> 
> Trying to merge it before 2.12 soft freeze (next Tuesday) still
> looks like a reasonable goal to me.  What do others think?

I've only really looked at the QAPI / QMP bits and they seem fine from
pov of libvirt's needs - just very minor comments. So not objection from
me on that area of the code.

Regards,
Daniel
Dr. David Alan Gilbert March 9, 2018, 10:12 a.m. UTC | #5
* Eduardo Habkost (ehabkost@redhat.com) wrote:
> On Thu, Mar 08, 2018 at 02:18:55PM -0600, Brijesh Singh wrote:
> > 
> > 
> > On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
> > > On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
> > >> Blacklist the following commands to fix the 'make check' failure.
> > >>
> > >> query-sev-launch-measure: it returns meaninful data only when we launch
> > >> SEV guest otherwise the command returns an error.
> > >>
> > >> query-sev: it return an error when SEV is not available on host (e.g non
> > >> X86 platform or KVM is disabled at the build time)
> > >>
> > >> query-sev-capabilities: it returns an error when SEV feature is not
> > >> available on host machine.
> > > We generally expect 'make check' to succeed on every single patch
> > > in a series, so that 'git bisect' doesn't break.
> > >
> > > So you should add each command to the blacklist in the same commit
> > > that introduced the failure in the first place.
> > 
> > 
> > Sure, I can quickly send the updated patch series to address your this
> > concern, but before spamming everyone's inbox I was wondering if I can
> > get some indication whether this series will make into 2.12 merge.
> > 
> > Paolo, Eduardo and Richard,
> > 
> > Most of the changes are in x86 directory hence any thought if you are
> > considering this series for 2.12 ? I have been testing the series with
> > and without SEV support and so far have not ran into any issue. if you
> > are not planning to pull this series in 2.12 then I will wait a bit
> > longer to get more feedback before sending the updates to address
> > Daniel's comment. thanks
> 
> Trying to merge it before 2.12 soft freeze (next Tuesday) still
> looks like a reasonable goal to me.  What do others think?

I've only looked at a few general comments and things but it looks like
it's getting there;  I don't think it's had many comments from the KVM
side yet.

Dave

> -- 
> Eduardo
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Paolo Bonzini March 13, 2018, 9:07 a.m. UTC | #6
On 09/03/2018 11:12, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost@redhat.com) wrote:
>> On Thu, Mar 08, 2018 at 02:18:55PM -0600, Brijesh Singh wrote:
>>>
>>>
>>> On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
>>>> On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
>>>>> Blacklist the following commands to fix the 'make check' failure.
>>>>>
>>>>> query-sev-launch-measure: it returns meaninful data only when we launch
>>>>> SEV guest otherwise the command returns an error.
>>>>>
>>>>> query-sev: it return an error when SEV is not available on host (e.g non
>>>>> X86 platform or KVM is disabled at the build time)
>>>>>
>>>>> query-sev-capabilities: it returns an error when SEV feature is not
>>>>> available on host machine.
>>>> We generally expect 'make check' to succeed on every single patch
>>>> in a series, so that 'git bisect' doesn't break.
>>>>
>>>> So you should add each command to the blacklist in the same commit
>>>> that introduced the failure in the first place.
>>>
>>>
>>> Sure, I can quickly send the updated patch series to address your this
>>> concern, but before spamming everyone's inbox I was wondering if I can
>>> get some indication whether this series will make into 2.12 merge.
>>>
>>> Paolo, Eduardo and Richard,
>>>
>>> Most of the changes are in x86 directory hence any thought if you are
>>> considering this series for 2.12 ? I have been testing the series with
>>> and without SEV support and so far have not ran into any issue. if you
>>> are not planning to pull this series in 2.12 then I will wait a bit
>>> longer to get more feedback before sending the updates to address
>>> Daniel's comment. thanks
>>
>> Trying to merge it before 2.12 soft freeze (next Tuesday) still
>> looks like a reasonable goal to me.  What do others think?
> 
> I've only looked at a few general comments and things but it looks like
> it's getting there;  I don't think it's had many comments from the KVM
> side yet.

The KVM side is a pretty linear use of the kernel API.  I'm not very
happy with the debug API for MemoryRegions (but it's not really
Brijesh's fault), so my plan would be to merge it without debug support.

Paolo
Brijesh Singh March 13, 2018, 11:21 a.m. UTC | #7
On 3/13/18 4:07 AM, Paolo Bonzini wrote:
> On 09/03/2018 11:12, Dr. David Alan Gilbert wrote:
>> * Eduardo Habkost (ehabkost@redhat.com) wrote:
>>> On Thu, Mar 08, 2018 at 02:18:55PM -0600, Brijesh Singh wrote:
>>>>
>>>> On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
>>>>> On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
>>>>>> Blacklist the following commands to fix the 'make check' failure.
>>>>>>
>>>>>> query-sev-launch-measure: it returns meaninful data only when we launch
>>>>>> SEV guest otherwise the command returns an error.
>>>>>>
>>>>>> query-sev: it return an error when SEV is not available on host (e.g non
>>>>>> X86 platform or KVM is disabled at the build time)
>>>>>>
>>>>>> query-sev-capabilities: it returns an error when SEV feature is not
>>>>>> available on host machine.
>>>>> We generally expect 'make check' to succeed on every single patch
>>>>> in a series, so that 'git bisect' doesn't break.
>>>>>
>>>>> So you should add each command to the blacklist in the same commit
>>>>> that introduced the failure in the first place.
>>>>
>>>> Sure, I can quickly send the updated patch series to address your this
>>>> concern, but before spamming everyone's inbox I was wondering if I can
>>>> get some indication whether this series will make into 2.12 merge.
>>>>
>>>> Paolo, Eduardo and Richard,
>>>>
>>>> Most of the changes are in x86 directory hence any thought if you are
>>>> considering this series for 2.12 ? I have been testing the series with
>>>> and without SEV support and so far have not ran into any issue. if you
>>>> are not planning to pull this series in 2.12 then I will wait a bit
>>>> longer to get more feedback before sending the updates to address
>>>> Daniel's comment. thanks
>>> Trying to merge it before 2.12 soft freeze (next Tuesday) still
>>> looks like a reasonable goal to me.  What do others think?
>> I've only looked at a few general comments and things but it looks like
>> it's getting there;  I don't think it's had many comments from the KVM
>> side yet.
> The KVM side is a pretty linear use of the kernel API.  I'm not very
> happy with the debug API for MemoryRegions (but it's not really
> Brijesh's fault), so my plan would be to merge it without debug support.

Thanks Paolo, I am working to drop the complete debug support and will
send series very soon. I need to run some quick smoke test to make sure
I don't break SEV guest support.
Paolo Bonzini March 13, 2018, 11:36 a.m. UTC | #8
On 13/03/2018 12:21, Brijesh Singh wrote:
>> The KVM side is a pretty linear use of the kernel API.  I'm not very
>> happy with the debug API for MemoryRegions (but it's not really
>> Brijesh's fault), so my plan would be to merge it without debug support.
> 
> Thanks Paolo, I am working to drop the complete debug support and will
> send series very soon. I need to run some quick smoke test to make sure
> I don't break SEV guest support.

Just test the pull request I'll send in less than an hour. :)

Paolo
diff mbox

Patch

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 22445d9ec258..7470c6b754bc 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -204,6 +204,11 @@  static bool query_is_blacklisted(const char *cmd)
         "query-gic-capabilities", /* arm */
         /* Success depends on target-specific build configuration: */
         "query-pci",              /* CONFIG_PCI */
+        /* Success depends on launching SEV guest */
+        "query-sev-launch-measure",
+        /* Success depends on Host or Hypervisor SEV support */
+        "query-sev",
+        "query-sev-capabilities",
         NULL
     };
     int i;