diff mbox series

[v2,06/12] vl: Factor configure_blockdev() out of main()

Message ID 20190307172401.29451-7-armbru@redhat.com
State New
Headers show
Series pc: Support firmware configuration with -blockdev | expand

Commit Message

Markus Armbruster March 7, 2019, 5:23 p.m. UTC
Signed-off-by: Markus Armbruster <armbru@redhat.com>

# Conflicts:
#	vl.c
---
 vl.c | 71 +++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 39 insertions(+), 32 deletions(-)

Comments

Philippe Mathieu-Daudé March 7, 2019, 5:46 p.m. UTC | #1
On 3/7/19 6:23 PM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> # Conflicts:
> #	vl.c

^ leftover?

> ---
>  vl.c | 71 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 39 insertions(+), 32 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 00cb8ea7ca..5a19d2a8ec 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1201,6 +1201,44 @@ typedef struct BlockdevOptionsQueueEntry {
>  
>  typedef QSIMPLEQ_HEAD(, BlockdevOptionsQueueEntry) BlockdevOptionsQueue;
>  
> +static void configure_blockdev(BlockdevOptionsQueue *bdo_queue,
> +                               MachineClass *machine_class, int snapshot)
> +{
> +    /* If the currently selected machine wishes to override the units-per-bus
> +     * property of its default HBA interface type, do so now. */
> +    if (machine_class->units_per_default_bus) {
> +        override_max_devs(machine_class->block_default_type,
> +                          machine_class->units_per_default_bus);
> +    }
> +
> +    /* open the virtual block devices */
> +    while (!QSIMPLEQ_EMPTY(bdo_queue)) {
> +        BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(bdo_queue);
> +
> +        QSIMPLEQ_REMOVE_HEAD(bdo_queue, entry);
> +        loc_push_restore(&bdo->loc);
> +        qmp_blockdev_add(bdo->bdo, &error_fatal);
> +        loc_pop(&bdo->loc);
> +        qapi_free_BlockdevOptions(bdo->bdo);
> +        g_free(bdo);
> +    }
> +    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
> +        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
> +                          NULL, NULL);
> +    }
> +    if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
> +                          &machine_class->block_default_type, &error_fatal)) {
> +        /* We printed help */
> +        exit(0);
> +    }
> +
> +    default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
> +                  CDROM_OPTS);
> +    default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
> +    default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
> +
> +}
> +
>  static QemuOptsList qemu_smp_opts = {
>      .name = "smp-opts",
>      .implied_opt_name = "cpus",
> @@ -4360,38 +4398,7 @@ int main(int argc, char **argv, char **envp)
>      ram_mig_init();
>      dirty_bitmap_mig_init();
>  
> -    /* If the currently selected machine wishes to override the units-per-bus
> -     * property of its default HBA interface type, do so now. */
> -    if (machine_class->units_per_default_bus) {
> -        override_max_devs(machine_class->block_default_type,
> -                          machine_class->units_per_default_bus);
> -    }
> -
> -    /* open the virtual block devices */
> -    while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
> -        BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(&bdo_queue);
> -
> -        QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
> -        loc_push_restore(&bdo->loc);
> -        qmp_blockdev_add(bdo->bdo, &error_fatal);
> -        loc_pop(&bdo->loc);
> -        qapi_free_BlockdevOptions(bdo->bdo);
> -        g_free(bdo);
> -    }
> -    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
> -        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
> -                          NULL, NULL);
> -    }
> -    if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
> -                          &machine_class->block_default_type, &error_fatal)) {
> -        /* We printed help */
> -        exit(0);
> -    }
> -
> -    default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
> -                  CDROM_OPTS);
> -    default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
> -    default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
> +    configure_blockdev(&bdo_queue, machine_class, snapshot);

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  
>      qemu_opts_foreach(qemu_find_opts("mon"),
>                        mon_init_func, NULL, &error_fatal);
>
Eric Blake March 7, 2019, 5:59 p.m. UTC | #2
On 3/7/19 11:23 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> # Conflicts:
> #	vl.c

How'd you get git to preserve the leading #? Generally, I find conflicts
details useful for cherry-picked backports, but pointless for rebased
patches intended as original upstream material. And git defaults to
stripping lines with leading # when composing a commit message. May be
worth cleaning up before the actual pull request.
Markus Armbruster March 7, 2019, 7:08 p.m. UTC | #3
Eric Blake <eblake@redhat.com> writes:

> On 3/7/19 11:23 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> 
>> # Conflicts:
>> #	vl.c
>
> How'd you get git to preserve the leading #? Generally, I find conflicts
> details useful for cherry-picked backports, but pointless for rebased
> patches intended as original upstream material. And git defaults to
> stripping lines with leading # when composing a commit message.

I've messed up too many commit message by having fill-paragraph flow a #
to the beginning of a line, so I added

    [commit]
            cleanup = scissors

to my .gitconfig.  I've been messing up commit messages with leftover
crap ever since, but leftover crap has proven less confusing to my
reviewers than missing lines.

>                                                                 May be
> worth cleaning up before the actual pull request.

Certainly.


git-commit(1):

       --cleanup=<mode>
           This option determines how the supplied commit message should be
           cleaned up before committing. The <mode> can be strip, whitespace,
           verbatim, scissors or default.

           strip
               Strip leading and trailing empty lines, trailing whitespace,
               commentary and collapse consecutive empty lines.

           whitespace
               Same as strip except #commentary is not removed.

           verbatim
               Do not change the message at all.

           scissors
               Same as whitespace except that everything from (and including)
               the line found below is truncated, if the message is to be
               edited. "#" can be customized with core.commentChar.

                   # ------------------------ >8 ------------------------

           default
               Same as strip if the message is to be edited. Otherwise
               whitespace.

           The default can be changed by the commit.cleanup configuration
           variable (see git-config(1)).
Laszlo Ersek March 8, 2019, 8:31 a.m. UTC | #4
On 03/07/19 20:08, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 3/7/19 11:23 AM, Markus Armbruster wrote:
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>
>>> # Conflicts:
>>> #	vl.c
>>
>> How'd you get git to preserve the leading #? Generally, I find conflicts
>> details useful for cherry-picked backports, but pointless for rebased
>> patches intended as original upstream material. And git defaults to
>> stripping lines with leading # when composing a commit message.
> 
> I've messed up too many commit message by having fill-paragraph flow a #
> to the beginning of a line, so I added
> 
>     [commit]
>             cleanup = scissors

I'm going to steal this now. :)

> 
> to my .gitconfig.  I've been messing up commit messages with leftover
> crap ever since, but leftover crap has proven less confusing to my
> reviewers than missing lines.
> 
>>                                                                 May be
>> worth cleaning up before the actual pull request.
> 
> Certainly.
> 
> 
> git-commit(1):
> 
>        --cleanup=<mode>
>            This option determines how the supplied commit message should be
>            cleaned up before committing. The <mode> can be strip, whitespace,
>            verbatim, scissors or default.
> 
>            strip
>                Strip leading and trailing empty lines, trailing whitespace,
>                commentary and collapse consecutive empty lines.
> 
>            whitespace
>                Same as strip except #commentary is not removed.
> 
>            verbatim
>                Do not change the message at all.
> 
>            scissors
>                Same as whitespace except that everything from (and including)
>                the line found below is truncated, if the message is to be
>                edited. "#" can be customized with core.commentChar.
> 
>                    # ------------------------ >8 ------------------------
> 
>            default
>                Same as strip if the message is to be edited. Otherwise
>                whitespace.
> 
>            The default can be changed by the commit.cleanup configuration
>            variable (see git-config(1)).
> 

Yeah I can tell this documentation was written by a programmer. The
documentation most likely follows the implementation closely (nested
"if"s?), with the "double except". Just try to expand the definition of
"scissors" without a mental stack overflow:

    Same as strip except #commentary is not removed except that
    everything from (and including) the line found below is truncated

Geez.

Anyway, thank you again for the tip!
Laszlo
diff mbox series

Patch

diff --git a/vl.c b/vl.c
index 00cb8ea7ca..5a19d2a8ec 100644
--- a/vl.c
+++ b/vl.c
@@ -1201,6 +1201,44 @@  typedef struct BlockdevOptionsQueueEntry {
 
 typedef QSIMPLEQ_HEAD(, BlockdevOptionsQueueEntry) BlockdevOptionsQueue;
 
+static void configure_blockdev(BlockdevOptionsQueue *bdo_queue,
+                               MachineClass *machine_class, int snapshot)
+{
+    /* If the currently selected machine wishes to override the units-per-bus
+     * property of its default HBA interface type, do so now. */
+    if (machine_class->units_per_default_bus) {
+        override_max_devs(machine_class->block_default_type,
+                          machine_class->units_per_default_bus);
+    }
+
+    /* open the virtual block devices */
+    while (!QSIMPLEQ_EMPTY(bdo_queue)) {
+        BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(bdo_queue);
+
+        QSIMPLEQ_REMOVE_HEAD(bdo_queue, entry);
+        loc_push_restore(&bdo->loc);
+        qmp_blockdev_add(bdo->bdo, &error_fatal);
+        loc_pop(&bdo->loc);
+        qapi_free_BlockdevOptions(bdo->bdo);
+        g_free(bdo);
+    }
+    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
+                          NULL, NULL);
+    }
+    if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
+                          &machine_class->block_default_type, &error_fatal)) {
+        /* We printed help */
+        exit(0);
+    }
+
+    default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
+                  CDROM_OPTS);
+    default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
+    default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
+
+}
+
 static QemuOptsList qemu_smp_opts = {
     .name = "smp-opts",
     .implied_opt_name = "cpus",
@@ -4360,38 +4398,7 @@  int main(int argc, char **argv, char **envp)
     ram_mig_init();
     dirty_bitmap_mig_init();
 
-    /* If the currently selected machine wishes to override the units-per-bus
-     * property of its default HBA interface type, do so now. */
-    if (machine_class->units_per_default_bus) {
-        override_max_devs(machine_class->block_default_type,
-                          machine_class->units_per_default_bus);
-    }
-
-    /* open the virtual block devices */
-    while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
-        BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(&bdo_queue);
-
-        QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
-        loc_push_restore(&bdo->loc);
-        qmp_blockdev_add(bdo->bdo, &error_fatal);
-        loc_pop(&bdo->loc);
-        qapi_free_BlockdevOptions(bdo->bdo);
-        g_free(bdo);
-    }
-    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
-        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
-                          NULL, NULL);
-    }
-    if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
-                          &machine_class->block_default_type, &error_fatal)) {
-        /* We printed help */
-        exit(0);
-    }
-
-    default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
-                  CDROM_OPTS);
-    default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
-    default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
+    configure_blockdev(&bdo_queue, machine_class, snapshot);
 
     qemu_opts_foreach(qemu_find_opts("mon"),
                       mon_init_func, NULL, &error_fatal);