diff mbox

[8/9] balloon: Factor out common "is balloon active" test

Message ID 1421171432-8733-9-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Jan. 13, 2015, 5:50 p.m. UTC
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 balloon.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

Eric Blake Jan. 14, 2015, 1:29 p.m. UTC | #1
On 01/13/2015 10:50 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  balloon.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/balloon.c b/balloon.c
> index 2884c2d..aa30617 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
>  static QEMUBalloonStatus *balloon_stat_fn;
>  static void *balloon_opaque;
>  
> +static int have_ballon(Error **errp)
> +{
> +    if (kvm_enabled() && !kvm_has_sync_mmu()) {
> +        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
> +        return 0;
> +    }
> +    if (!balloon_event_fn) {
> +        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
> +        return 0;
> +    }
> +    return 1;

Another case where I would have used bool, true, false.

Either way,
Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Jan. 14, 2015, 2:34 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 01/13/2015 10:50 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  balloon.c | 29 +++++++++++++++--------------
>>  1 file changed, 15 insertions(+), 14 deletions(-)
>> 
>> diff --git a/balloon.c b/balloon.c
>> index 2884c2d..aa30617 100644
>> --- a/balloon.c
>> +++ b/balloon.c
>> @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
>>  static QEMUBalloonStatus *balloon_stat_fn;
>>  static void *balloon_opaque;
>>  
>> +static int have_ballon(Error **errp)
>> +{
>> +    if (kvm_enabled() && !kvm_has_sync_mmu()) {
>> +        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
>> +        return 0;
>> +    }
>> +    if (!balloon_event_fn) {
>> +        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
>> +        return 0;
>> +    }
>> +    return 1;
>
> Another case where I would have used bool, true, false.

Me too now that you mention it %-)

> Either way,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
Dr. David Alan Gilbert Feb. 10, 2015, 6:39 p.m. UTC | #3
* Markus Armbruster (armbru@redhat.com) wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  balloon.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/balloon.c b/balloon.c
> index 2884c2d..aa30617 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
>  static QEMUBalloonStatus *balloon_stat_fn;
>  static void *balloon_opaque;
>  
> +static int have_ballon(Error **errp)

                       ^

An 'o' escaped from your balloon.

Dave

> +{
> +    if (kvm_enabled() && !kvm_has_sync_mmu()) {
> +        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
> +        return 0;
> +    }
> +    if (!balloon_event_fn) {
> +        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
> +        return 0;
> +    }
> +    return 1;
> +}
> +
>  int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
>                               QEMUBalloonStatus *stat_func, void *opaque)
>  {
> @@ -66,13 +79,7 @@ BalloonInfo *qmp_query_balloon(Error **errp)
>  {
>      BalloonInfo *info;
>  
> -    if (kvm_enabled() && !kvm_has_sync_mmu()) {
> -        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
> -        return NULL;
> -    }
> -
> -    if (!balloon_stat_fn) {
> -        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
> +    if (!have_ballon(errp)) {
>          return NULL;
>      }
>  
> @@ -83,8 +90,7 @@ BalloonInfo *qmp_query_balloon(Error **errp)
>  
>  void qmp_balloon(int64_t target, Error **errp)
>  {
> -    if (kvm_enabled() && !kvm_has_sync_mmu()) {
> -        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
> +    if (!have_ballon(errp)) {
>          return;
>      }
>  
> @@ -92,11 +98,6 @@ void qmp_balloon(int64_t target, Error **errp)
>          error_set(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size");
>          return;
>      }
> -    
> -    if (!balloon_event_fn) {
> -        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
> -        return;
> -    }
>  
>      trace_balloon_event(balloon_opaque, target);
>      balloon_event_fn(balloon_opaque, target);
> -- 
> 1.9.3
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Eric Blake Feb. 10, 2015, 10:42 p.m. UTC | #4
On 02/10/2015 11:39 AM, Dr. David Alan Gilbert wrote:
> * Markus Armbruster (armbru@redhat.com) wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  balloon.c | 29 +++++++++++++++--------------
>>  1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/balloon.c b/balloon.c
>> index 2884c2d..aa30617 100644
>> --- a/balloon.c
>> +++ b/balloon.c
>> @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
>>  static QEMUBalloonStatus *balloon_stat_fn;
>>  static void *balloon_opaque;
>>  
>> +static int have_ballon(Error **errp)
> 
>                        ^
> 
> An 'o' escaped from your balloon.

Oops - it also escaped from my R-b.  I've sent the obvious patch, as
penance for my "deflated ego" :)
Markus Armbruster Feb. 11, 2015, 12:12 p.m. UTC | #5
Eric Blake <eblake@redhat.com> writes:

> On 02/10/2015 11:39 AM, Dr. David Alan Gilbert wrote:
>> * Markus Armbruster (armbru@redhat.com) wrote:
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  balloon.c | 29 +++++++++++++++--------------
>>>  1 file changed, 15 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/balloon.c b/balloon.c
>>> index 2884c2d..aa30617 100644
>>> --- a/balloon.c
>>> +++ b/balloon.c
>>> @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
>>>  static QEMUBalloonStatus *balloon_stat_fn;
>>>  static void *balloon_opaque;
>>>  
>>> +static int have_ballon(Error **errp)
>> 
>>                        ^
>> 
>> An 'o' escaped from your balloon.
>
> Oops - it also escaped from my R-b.  I've sent the obvious patch, as
> penance for my "deflated ego" :)

Thank you both!
diff mbox

Patch

diff --git a/balloon.c b/balloon.c
index 2884c2d..aa30617 100644
--- a/balloon.c
+++ b/balloon.c
@@ -36,6 +36,19 @@  static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
 static void *balloon_opaque;
 
+static int have_ballon(Error **errp)
+{
+    if (kvm_enabled() && !kvm_has_sync_mmu()) {
+        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
+        return 0;
+    }
+    if (!balloon_event_fn) {
+        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
+        return 0;
+    }
+    return 1;
+}
+
 int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
                              QEMUBalloonStatus *stat_func, void *opaque)
 {
@@ -66,13 +79,7 @@  BalloonInfo *qmp_query_balloon(Error **errp)
 {
     BalloonInfo *info;
 
-    if (kvm_enabled() && !kvm_has_sync_mmu()) {
-        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
-        return NULL;
-    }
-
-    if (!balloon_stat_fn) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
+    if (!have_ballon(errp)) {
         return NULL;
     }
 
@@ -83,8 +90,7 @@  BalloonInfo *qmp_query_balloon(Error **errp)
 
 void qmp_balloon(int64_t target, Error **errp)
 {
-    if (kvm_enabled() && !kvm_has_sync_mmu()) {
-        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
+    if (!have_ballon(errp)) {
         return;
     }
 
@@ -92,11 +98,6 @@  void qmp_balloon(int64_t target, Error **errp)
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size");
         return;
     }
-    
-    if (!balloon_event_fn) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
-        return;
-    }
 
     trace_balloon_event(balloon_opaque, target);
     balloon_event_fn(balloon_opaque, target);