[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 06/14] hw/i386/vmport: Define enum for all commands

From: Liran Alon
Subject: Re: [PATCH 06/14] hw/i386/vmport: Define enum for all commands
Date: Tue, 10 Mar 2020 13:37:40 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 10/03/2020 13:23, Michael S. Tsirkin wrote:
On Tue, Mar 10, 2020 at 01:16:51PM +0200, Liran Alon wrote:
On 10/03/2020 11:28, Michael S. Tsirkin wrote:
On Tue, Mar 10, 2020 at 01:54:03AM +0200, Liran Alon wrote:
No functional change.

Defining an enum for all VMPort commands have the following advantages:
* It gets rid of the error-prone requirement to update VMPORT_ENTRIES
when new VMPort commands are added to QEMU.
* It makes it clear to know by looking at one place at the source, what
are all the VMPort commands supported by QEMU.

Reviewed-by: Nikita Leshenko <address@hidden>
Signed-off-by: Liran Alon <address@hidden>

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index d5ac76d54e1f..7f15a01137b1 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -138,12 +138,21 @@ GSIState *pc_gsi_create(qemu_irq **irqs, bool 
   #define TYPE_VMPORT "vmport"
   typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address);
+typedef enum {
+    VMPORT_CMD_GETVERSION       = 10,
+    VMPORT_CMD_GETRAMSIZE       = 20,
+} VMPortCommand;
Please don't, let's leave pc.h alone. If you must add a new header for
vmport/vmmouse and put this stuff there.
As you can see, pc.h already contains definitions which are specific to
vmport. E.g. TYPE_VMPORT, VMPortReadFunc(), vmport_register(),
vmmouse_get_data(), vmmouse_set_data(). Adding this enum is not what makes
the difference.
It is possible to create a new vmport.h header file but it's not really
related to this patch. It's just general refactoring. I can do that in v2 if
you think it's appropriate.

Well I just don't want lots of enums in pc.h

This is the only one which is global, and makes sense as it's directly related to vmport_register() exposed API.
Similar to how the VMPortReadFunc typedef is put in here.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]