[Qemu-devel] [PATCH] Add -no-virtio-balloon command-line option

[Qemu-devel] [PATCH] Add -no-virtio-balloon command-line option

[Eduardo Habkost]

This new option may be used to disable the virtio-balloon device.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pc.c         |    2 +-
 qemu-options.hx |    9 +++++++++
 sysemu.h        |    1 +
 vl.c            |    4 ++++
 4 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 0934778..aa92576 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1152,7 +1152,7 @@ static void pc_init1(ram_addr_t ram_size,
     }
 
     /* Add virtio balloon device */
-    if (pci_enabled) {
+    if (pci_enabled && !no_virtio_balloon) {
         pci_create_simple(pci_bus, -1, "virtio-balloon-pci");
     }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index fa549c3..cb68e2a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -681,6 +681,15 @@ Disable HPET support.
 ETEXI
 
 #ifdef TARGET_I386
+DEF("no-virtio-balloon", 0, QEMU_OPTION_no_virtio_balloon,
+    "-no-virtio-baloon disable virtio balloon device\n")
+#endif
+STEXI
+@item -no-virtio-balloon
+Disable virtio-balloon device.
+ETEXI
+
+#ifdef TARGET_I386
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
     "-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,data=file1[:file2]...]\n"
     "                ACPI table description\n")
diff --git a/sysemu.h b/sysemu.h
index 658aeec..4a0ddcb 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -116,6 +116,7 @@ extern int win2k_install_hack;
 extern int rtc_td_hack;
 extern int alt_grab;
 extern int usb_enabled;
+extern int no_virtio_balloon;
 extern int smp_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
diff --git a/vl.c b/vl.c
index 69a9f91..9f7216f 100644
--- a/vl.c
+++ b/vl.c
@@ -244,6 +244,7 @@ int smp_cpus = 1;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
+int no_virtio_balloon = 0;
 int fd_bootchk = 1;
 int no_reboot = 0;
 int no_shutdown = 0;
@@ -5552,6 +5553,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_no_hpet:
                 no_hpet = 1;
                 break;
+            case QEMU_OPTION_no_virtio_balloon:
+                no_virtio_balloon = 1;
+                break;
 #endif
             case QEMU_OPTION_no_reboot:
                 no_reboot = 1;
-- 
1.6.3.rc4.29.g8146

[Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Anthony Liguori]

Eduardo Habkost wrote:
> This new option may be used to disable the virtio-balloon device.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/pc.c         |    2 +-
>  qemu-options.hx |    9 +++++++++
>  sysemu.h        |    1 +
>  vl.c            |    4 ++++
>  4 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 0934778..aa92576 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1152,7 +1152,7 @@ static void pc_init1(ram_addr_t ram_size,
>      }
>  
>      /* Add virtio balloon device */
> -    if (pci_enabled) {
> +    if (pci_enabled && !no_virtio_balloon) {
>          pci_create_simple(pci_bus, -1, "virtio-balloon-pci");
>      }
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index fa549c3..cb68e2a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -681,6 +681,15 @@ Disable HPET support.
>  ETEXI
>  
>  #ifdef TARGET_I386
> +DEF("no-virtio-balloon", 0, QEMU_OPTION_no_virtio_balloon,
> +    "-no-virtio-baloon disable virtio balloon device\n")
>   
Missing 'l'.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Eduardo Habkost]

On Wed, Jun 10, 2009 at 02:25:05PM -0500, Anthony Liguori wrote:
> Eduardo Habkost wrote:
>> This new option may be used to disable the virtio-balloon device.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  hw/pc.c         |    2 +-
>>  qemu-options.hx |    9 +++++++++
>>  sysemu.h        |    1 +
>>  vl.c            |    4 ++++
>>  4 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 0934778..aa92576 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -1152,7 +1152,7 @@ static void pc_init1(ram_addr_t ram_size,
>>      }
>>       /* Add virtio balloon device */
>> -    if (pci_enabled) {
>> +    if (pci_enabled && !no_virtio_balloon) {
>>          pci_create_simple(pci_bus, -1, "virtio-balloon-pci");
>>      }
>>  diff --git a/qemu-options.hx b/qemu-options.hx
>> index fa549c3..cb68e2a 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -681,6 +681,15 @@ Disable HPET support.
>>  ETEXI
>>   #ifdef TARGET_I386
>> +DEF("no-virtio-balloon", 0, QEMU_OPTION_no_virtio_balloon,
>> +    "-no-virtio-baloon disable virtio balloon device\n")
>>   
> Missing 'l'.

Oops! Fixed version below.


>From 1276e7dca28cfcf5eae1ed69fe5a9ae0853647c6 Mon Sep 17 00:00:00 2001
From: Eduardo Habkost <ehabkost@redhat.com>
Date: Tue, 9 Jun 2009 17:06:58 -0300
Subject: [PATCH] Add -no-virtio-balloon command-line option

This new option may be used to disable the virtio-balloon device.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pc.c         |    2 +-
 qemu-options.hx |    9 +++++++++
 sysemu.h        |    1 +
 vl.c            |    4 ++++
 4 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 0934778..aa92576 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1152,7 +1152,7 @@ static void pc_init1(ram_addr_t ram_size,
     }
 
     /* Add virtio balloon device */
-    if (pci_enabled) {
+    if (pci_enabled && !no_virtio_balloon) {
         pci_create_simple(pci_bus, -1, "virtio-balloon-pci");
     }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index fa549c3..9d5e05a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -681,6 +681,15 @@ Disable HPET support.
 ETEXI
 
 #ifdef TARGET_I386
+DEF("no-virtio-balloon", 0, QEMU_OPTION_no_virtio_balloon,
+    "-no-virtio-balloon disable virtio balloon device\n")
+#endif
+STEXI
+@item -no-virtio-balloon
+Disable virtio-balloon device.
+ETEXI
+
+#ifdef TARGET_I386
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
     "-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,data=file1[:file2]...]\n"
     "                ACPI table description\n")
diff --git a/sysemu.h b/sysemu.h
index 658aeec..4a0ddcb 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -116,6 +116,7 @@ extern int win2k_install_hack;
 extern int rtc_td_hack;
 extern int alt_grab;
 extern int usb_enabled;
+extern int no_virtio_balloon;
 extern int smp_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
diff --git a/vl.c b/vl.c
index 69a9f91..9f7216f 100644
--- a/vl.c
+++ b/vl.c
@@ -244,6 +244,7 @@ int smp_cpus = 1;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
+int no_virtio_balloon = 0;
 int fd_bootchk = 1;
 int no_reboot = 0;
 int no_shutdown = 0;
@@ -5552,6 +5553,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_no_hpet:
                 no_hpet = 1;
                 break;
+            case QEMU_OPTION_no_virtio_balloon:
+                no_virtio_balloon = 1;
+                break;
 #endif
             case QEMU_OPTION_no_reboot:
                 no_reboot = 1;
-- 
1.6.3.rc4.29.g8146

-- 
Eduardo

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Gerd Hoffmann]

>
> This new option may be used to disable the virtio-balloon device.
>

ACK, looks good.

And I can drop my lazy patch which just comments out virtio-balloon in 
the code ;)

cheers,
   Gerd

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Dor Laor]

Gerd Hoffmann wrote:
>>
>> This new option may be used to disable the virtio-balloon device.
>>
>
> ACK, looks good.
>
> And I can drop my lazy patch which just comments out virtio-balloon in 
> the code ;)

Actually, I rather have the balloon disabled by default. It's like any 
other pci device which needs
explicit command line specification. Why consume a pci slot if not 
implicitly required?
>
> cheers,
>   Gerd
>
>
>

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Markus Armbruster]

Dor Laor <dlaor@redhat.com> writes:

> Gerd Hoffmann wrote:
>>>
>>> This new option may be used to disable the virtio-balloon device.
>>>
>>
>> ACK, looks good.
>>
>> And I can drop my lazy patch which just comments out virtio-balloon
>> in the code ;)
>
> Actually, I rather have the balloon disabled by default. It's like any
> other pci device which needs
> explicit command line specification. Why consume a pci slot if not
> implicitly required?

I agree.  The command line is plenty magic without implicitly added
PCI devices.

Besides, negative options -no-virtio-balloon lead to ugly
double-negatives like if (!no_virtio_balloon).

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Anthony Liguori]

Markus Armbruster wrote:
> Dor Laor <dlaor@redhat.com> writes:
>
>   
>> Gerd Hoffmann wrote:
>>     
>>>> This new option may be used to disable the virtio-balloon device.
>>>>
>>>>         
>>> ACK, looks good.
>>>
>>> And I can drop my lazy patch which just comments out virtio-balloon
>>> in the code ;)
>>>       
>> Actually, I rather have the balloon disabled by default. It's like any
>> other pci device which needs
>> explicit command line specification. Why consume a pci slot if not
>> implicitly required?
>>     
>
> I agree.  The command line is plenty magic without implicitly added
> PCI devices.
>
> Besides, negative options -no-virtio-balloon lead to ugly
> double-negatives like if (!no_virtio_balloon).
>   

It does no harm to add the device by default so why force the user to 
explicitly enable it?

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Dor Laor]

Anthony Liguori wrote:
> Markus Armbruster wrote:
>> Dor Laor <dlaor@redhat.com> writes:
>>
>>  
>>> Gerd Hoffmann wrote:
>>>    
>>>>> This new option may be used to disable the virtio-balloon device.
>>>>>
>>>>>         
>>>> ACK, looks good.
>>>>
>>>> And I can drop my lazy patch which just comments out virtio-balloon
>>>> in the code ;)
>>>>       
>>> Actually, I rather have the balloon disabled by default. It's like any
>>> other pci device which needs
>>> explicit command line specification. Why consume a pci slot if not
>>> implicitly required?
>>>     
>>
>> I agree.  The command line is plenty magic without implicitly added
>> PCI devices.
>>
>> Besides, negative options -no-virtio-balloon lead to ugly
>> double-negatives like if (!no_virtio_balloon).
>>   
>
> It does no harm to add the device by default so why force the user to 
> explicitly enable it?

It consumes one of the 32 pci devices we have today. It's the same for 
not having -usb as default
or any other type of device. Soon we'll need pci_addr= along with it 
anyway ;)

Dor

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Anthony Liguori]

Dor Laor wrote:
> Anthony Liguori wrote:
>> Markus Armbruster wrote:
>>> Dor Laor <dlaor@redhat.com> writes:
>>>
>>>  
>>>> Gerd Hoffmann wrote:
>>>>   
>>>>>> This new option may be used to disable the virtio-balloon device.
>>>>>>
>>>>>>         
>>>>> ACK, looks good.
>>>>>
>>>>> And I can drop my lazy patch which just comments out virtio-balloon
>>>>> in the code ;)
>>>>>       
>>>> Actually, I rather have the balloon disabled by default. It's like any
>>>> other pci device which needs
>>>> explicit command line specification. Why consume a pci slot if not
>>>> implicitly required?
>>>>     
>>>
>>> I agree.  The command line is plenty magic without implicitly added
>>> PCI devices.
>>>
>>> Besides, negative options -no-virtio-balloon lead to ugly
>>> double-negatives like if (!no_virtio_balloon).
>>>   
>>
>> It does no harm to add the device by default so why force the user to 
>> explicitly enable it?
>
> It consumes one of the 32 pci devices we have today. It's the same for 
> not having -usb as default
> or any other type of device. Soon we'll need pci_addr= along with it 
> anyway ;)

IMHO, -usb should be default.  It wasn't made default when it was 
introduced because it was known to be buggy.

For management tools, explicitly disabling something is easy enough.  
For humans, I think wanting features is more common than needing more 
than 32 PCI slots.

Regards,

Anthony Liguori

> Dor
>

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> Markus Armbruster wrote:
>> Dor Laor <dlaor@redhat.com> writes:
>>
>>   
>>> Gerd Hoffmann wrote:
>>>     
>>>>> This new option may be used to disable the virtio-balloon device.
>>>>>
>>>>>         
>>>> ACK, looks good.
>>>>
>>>> And I can drop my lazy patch which just comments out virtio-balloon
>>>> in the code ;)
>>>>       
>>> Actually, I rather have the balloon disabled by default. It's like any
>>> other pci device which needs
>>> explicit command line specification. Why consume a pci slot if not
>>> implicitly required?
>>>     
>>
>> I agree.  The command line is plenty magic without implicitly added
>> PCI devices.
>>
>> Besides, negative options -no-virtio-balloon lead to ugly
>> double-negatives like if (!no_virtio_balloon).
>>   
>
> It does no harm to add the device by default so why force the user to
> explicitly enable it?
>
> Regards,
>
> Anthony Liguori

I don't care whether it's on or off by default.  I just object to the
ugly -no-virtio-balloon.

We need to do (at least) two things to the virtio-balloon device:
suppress it, and control its PCI address.  We can do both with one
option, but it better not be called -no-virtio-balloon then.

Options controlling devices should always be extensible to accept
arbitrary name=value arguments.

Re: [Qemu-devel] Re: [PATCH] Add -no-virtio-balloon command-line option

[Anthony Liguori]

Markus Armbruster wrote:
> Anthony Liguori <anthony@codemonkey.ws> writes:
>
>   
>> Markus Armbruster wrote:
>>     
>>> Dor Laor <dlaor@redhat.com> writes:
>>>
>>>   
>>>       
>>>> Gerd Hoffmann wrote:
>>>>     
>>>>         
>>>>>> This new option may be used to disable the virtio-balloon device.
>>>>>>
>>>>>>         
>>>>>>             
>>>>> ACK, looks good.
>>>>>
>>>>> And I can drop my lazy patch which just comments out virtio-balloon
>>>>> in the code ;)
>>>>>       
>>>>>           
>>>> Actually, I rather have the balloon disabled by default. It's like any
>>>> other pci device which needs
>>>> explicit command line specification. Why consume a pci slot if not
>>>> implicitly required?
>>>>     
>>>>         
>>> I agree.  The command line is plenty magic without implicitly added
>>> PCI devices.
>>>
>>> Besides, negative options -no-virtio-balloon lead to ugly
>>> double-negatives like if (!no_virtio_balloon).
>>>   
>>>       
>> It does no harm to add the device by default so why force the user to
>> explicitly enable it?
>>
>> Regards,
>>
>> Anthony Liguori
>>     
>
> I don't care whether it's on or off by default.  I just object to the
> ugly -no-virtio-balloon.
>
> We need to do (at least) two things to the virtio-balloon device:
> suppress it, and control its PCI address.  We can do both with one
> option, but it better not be called -no-virtio-balloon then.
>
> Options controlling devices should always be extensible to accept
> arbitrary name=value arguments.
>   
It's not in a released version of QEMU so we can remove the option and 
change it's syntax.

-balloon none
-balloon virtio
-balloon virtio,addr=00:01.3

Could be a reasonable approach.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH] Replace -no-virtio-balloon by -balloon

[Markus Armbruster]

We want to do (at least) two things to the virtio-balloon device:
suppress it, and control its PCI address.  Option -no-virtio-balloon
lets us do only the former.  To get the latter, replace
-no-virtio-balloon with

    -balloon none   disable balloon device
    -balloon virtio[,addr=str]
                    enable virtio balloon device (default)

Syntax suggested by Anthony Liguori.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pc.c         |    5 +++--
 qemu-options.hx |   13 +++++++++----
 sysemu.h        |    3 ++-
 vl.c            |   33 ++++++++++++++++++++++++++++++---
 4 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 86e5cfe..6a314a7 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1166,8 +1166,9 @@ static void pc_init1(ram_addr_t ram_size,
     }
 
     /* Add virtio balloon device */
-    if (pci_enabled && !no_virtio_balloon) {
-        pci_create_simple(pci_bus, -1, "virtio-balloon-pci");
+    if (pci_enabled && virtio_balloon) {
+        pci_dev = pci_create("virtio-balloon-pci", virtio_balloon_devaddr);
+        qdev_init(&pci_dev->qdev);
     }
 
     /* Add virtio console devices */
diff --git a/qemu-options.hx b/qemu-options.hx
index 503da33..153ad7c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -684,12 +684,17 @@ Disable HPET support.
 ETEXI
 
 #ifdef TARGET_I386
-DEF("no-virtio-balloon", 0, QEMU_OPTION_no_virtio_balloon,
-    "-no-virtio-balloon disable virtio balloon device\n")
+DEF("balloon", HAS_ARG, QEMU_OPTION_balloon,
+    "-balloon none   disable balloon device\n"
+    "-balloon virtio[,addr=str]\n"
+    "                enable virtio balloon device (default)\n")
 #endif
 STEXI
-@item -no-virtio-balloon
-Disable virtio-balloon device.
+@item -balloon none
+Disable balloon device.
+@item -balloon virtio[,addr=@var{addr}]
+Enable virtio balloon device (default), optionally with PCI address
+@var{addr}.
 ETEXI
 
 #ifdef TARGET_I386
diff --git a/sysemu.h b/sysemu.h
index d77de09..f6ab3e8 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -116,7 +116,8 @@ extern int win2k_install_hack;
 extern int rtc_td_hack;
 extern int alt_grab;
 extern int usb_enabled;
-extern int no_virtio_balloon;
+extern int virtio_balloon;
+extern const char *virtio_balloon_devaddr;
 extern int smp_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
diff --git a/vl.c b/vl.c
index 60a00e1..5b53baa 100644
--- a/vl.c
+++ b/vl.c
@@ -244,7 +244,8 @@ int smp_cpus = 1;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
-int no_virtio_balloon = 0;
+int virtio_balloon = 1;
+const char *virtio_balloon_devaddr;
 int fd_bootchk = 1;
 int no_reboot = 0;
 int no_shutdown = 0;
@@ -4769,6 +4770,29 @@ static void select_vgahw (const char *p)
     }
 }
 
+#ifdef TARGET_I386
+static int balloon_parse(const char *arg)
+{
+    char buf[128];
+    const char *p;
+
+    if (!strcmp(arg, "none")) {
+        virtio_balloon = 0;
+    } else if (!strncmp(arg, "virtio", 6)) {
+        virtio_balloon = 1;
+        if (arg[6] == ',')  {
+            p = arg + 7;
+            if (get_param_value(buf, sizeof(buf), "addr", p)) {
+                virtio_balloon_devaddr = strdup(buf);
+            }
+        }
+    } else {
+        return -1;
+    }
+    return 0;
+}
+#endif
+
 #ifdef _WIN32
 static BOOL WINAPI qemu_ctrl_handler(DWORD type)
 {
@@ -5585,8 +5609,11 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_no_hpet:
                 no_hpet = 1;
                 break;
-            case QEMU_OPTION_no_virtio_balloon:
-                no_virtio_balloon = 1;
+            case QEMU_OPTION_balloon:
+                if (balloon_parse(optarg) < 0) {
+                    fprintf(stderr, "Unknown -balloon argument %s\n", optarg);
+                    exit(1);
+                }
                 break;
 #endif
             case QEMU_OPTION_no_reboot:
-- 
1.6.2.5