qga: report serial number and disk node
cover qga: report serial number and disk node
Patches 1 and 3 are bug-fixes of existing code
Patch 2 is optional and contains debug messages -- the windows QGA seriously lacks debug messages...
Patch 4 adds reporting of disk serial number and device node of the disk.
Tomáš Golembiovský (4):
build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI
qga: win32: add debugging information
qga: win32: fix crashes when PCI info cannot be retrived
qga: report serial ID and device node
configure | 3 +- qga/commands-posix.c | 23 ++++++++++ qga/commands-win32.c | 105 ++++++++++++++++++++++++++++++++++--------- qga/qapi-schema.json | 4 +- 4 files changed, 111 insertions(+), 24 deletions(-)
Signature · 1 line
2.18.0
PATCH 1/4 build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI
There was inconsistency between commits:
50cbebb9a3 configure: add configure check for ntdddisk.h a3ef3b2272 qga: added bus type and disk location path
The first commit added #define CONFIG_QGA_NTDDDISK but the second commit expected the name to be CONFIG_QGA_NTDDSCSI. As a result the code in second patch was never used.
Renaming the option to CONFIG_QGA_NTDDSCSI to match the name of header file that is being checked for.
--- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
configure +1 -1
@@ -6111,7 +6111,7 @@ if test "$mingw32" = "yes" ; then echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak fi if test "$guest_agent_ntddscsi" = "yes" ; then - echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak + echo "CONFIG_QGA_NTDDSCSI=y" >> $config_host_mak fi if test "$guest_agent_msi" = "yes"; then echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak
Signature · 1 line
2.18.0
PATCH 2/4 qga: win32: add debugging information
In reply to Tomáš GolembiovskýThe windows code generaly lacks debug information (compared to posix code). This patch adds some related to HW info in guest-get-fsinfo command.
--- qga/commands-win32.c | 9 +++++++++ 1 file changed, 9 insertions(+)
qga/commands-win32.c +9 -0
@@ -498,6 +498,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) goto out; } +g_debug ("enumerating devices" ); dev_info_data.cbSize =sizeof (SP_DEVINFO_DATA );for (i =0 ;SetupDiEnumDeviceInfo (dev_info, i, &dev_info_data); i++) {DWORD addr, bus, slot, func, dev, data, size2;
@@ -522,6 +523,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)if (g_strcmp0 (buffer, dev_name)) {continue ; } +g_debug ("found device %s" , dev_name);/* There is no need to allocate buffer in the next functions. The size * is known and ULONG according to
@@ -530,6 +532,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)*/ if (!SetupDiGetDeviceRegistryProperty (dev_info, &dev_info_data,SPDRP_BUSNUMBER , &data, (PBYTE )&bus, size,NULL )) { +g_debug ("failed to get bus" );break ; }
@@ -537,6 +540,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)* transformed into device function and number */ if (!SetupDiGetDeviceRegistryProperty (dev_info, &dev_info_data,SPDRP_ADDRESS , &data, (PBYTE )&addr, size,NULL )) { +g_debug ("failed to get address" );break ; }
@@ -544,6 +548,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)* This number is typically a user-perceived slot number. */ if (!SetupDiGetDeviceRegistryProperty (dev_info, &dev_info_data,SPDRP_UI_NUMBER , &data, (PBYTE )&slot, size,NULL )) { +g_debug ("failed to get slot" );break ; }
@@ -608,6 +613,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp) scsi_ad = &addr;char *name =g_strndup (guid,strlen (guid)-1 ); +g_debug ("getting disk info for: %s" , name); vol_h =CreateFile (name,0 ,FILE_SHARE_READ ,NULL ,OPEN_EXISTING ,0 ,NULL );if (vol_h ==INVALID_HANDLE_VALUE ) {
@@ -615,6 +621,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp) goto out_free; } +g_debug ("getting bus type" ); bus =get_disk_bus_type (vol_h, errp);if (bus <0 ) { goto out_close;
@@ -622,6 +629,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp) disk =g_malloc0 (sizeof (*disk)); disk->bus_type =find_bus_type (bus); +g_debug ("bus type %d" , disk->bus_type);if (bus == BusTypeScsi || bus == BusTypeAta || bus ==BusTypeRAID #if (_WIN32_WINNT >= 0x0600) /* This bus type is not supported before Windows Server 2003 SP1 */
@@ -631,6 +639,7 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)/* We are able to use the same ioctls for different bus types * according to Microsoft docs * https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx */ +g_debug ("getting pci-controller info" );if (DeviceIoControl (vol_h,IOCTL_SCSI_GET_ADDRESS ,NULL ,0 , scsi_ad,sizeof (SCSI_ADDRESS ), &len,NULL )) { disk->unit = addr.Lun;
Signature · 1 line
2.18.0
PATCH 3/4 qga: win32: fix crashes when PCI info cannot be retrived
In reply to Tomáš GolembiovskýThe guest-get-fsinfo command collects also information about PCI controller where the disk is attached. When this fails for some reasons it tries to return just the partial information. However in certain cases the pointer to the structure was not initialized and was set to NULL. This breaks the serializer and lead to crasehs of the guest agent.
--- qga/commands-win32.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
qga/commands-win32.c +22 -5
@@ -642,15 +642,32 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)g_debug ("getting pci-controller info" );if (DeviceIoControl (vol_h,IOCTL_SCSI_GET_ADDRESS ,NULL ,0 , scsi_ad,sizeof (SCSI_ADDRESS ), &len,NULL )) { +Error *local_err =NULL ; disk->unit = addr.Lun; disk->target = addr.TargetId; disk->bus = addr.PathId; - disk->pci_controller =get_pci_info (name, errp); +g_debug ("unit=%lld target=%lld bus=%lld" , + disk->unit, disk->target, disk->bus); + disk->pci_controller =get_pci_info (name, &local_err); + +if (local_err) { +slog ("failed to get PCI controller info: %s" , +error_get_pretty (local_err)); +error_free (local_err); + }else if (disk->pci_controller !=NULL ) { +g_debug ("pci: domain=%lld bus=%lld slot=%lld function=%lld" , + disk->pci_controller->domain, + disk->pci_controller->bus, + disk->pci_controller->slot, + disk->pci_controller->function); + } } -/* We do not set error in this case, because we still have enough -* information about volume. */ - }else { - disk->pci_controller =NULL ; + } +/* We do not set error in case pci_controller is NULL, because we still +* have enough information about volume. */ +if (disk->pci_controller ==NULL ) { +g_debug ("no PCI controller info" ); + disk->pci_controller =g_malloc0 (sizeof (GuestPCIAddress)); } list =g_malloc0 (sizeof (*list));
Signature · 1 line
2.18.0
Re: [Qemu-devel] [PATCH 3/4] qga: win32: fix crashes when PCI info cannot be retrived
Quoted 5 lines · Tomáš Golembiovský
The guest-get-fsinfo command collects also information about PCI controller where the disk is attached. When this fails for some reasons it tries to return just the partial information. However in certain cases the pointer to the structure was not initialized and was set to NULL. This breaks the serializer and lead to crasehs of the guest agent.
s/lead to crasehs/leads to a crash/
Quoted 6 lines
Signed-off-by Tomáš Golembiovský <[email protected]>--- qga/commands-win32.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH 4/4 qga: report serial ID and device node
In reply to Tomáš GolembiovskýOn Linux the functionality depends on libudev.
Example from Linux:
{
"name": "dm-2",
"mountpoint": "/",
...
"disk": [
{
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
"dev": "/dev/sda2",
...
}
],
}--- configure | 1 + qga/commands-posix.c | 23 ++++++++++++++ qga/commands-win32.c | 73 ++++++++++++++++++++++++++++++++------------ qga/qapi-schema.json | 4 ++- 4 files changed, 81 insertions(+), 20 deletions(-)
configure +1 -0
@@ -871,6 +871,7 @@ Linux) vhost_vsock="yes" QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES" supported_os="yes" + libs_qga="$($pkg_config --libs libudev) $libs_qga" ;; esac
qga/commands-posix.c +23 -0
@@ -47,6 +47,7 @@ extern char **environ;#include <sys/socket.h> #include <net/if.h> #include <sys/statvfs.h> +#include <libudev.h> #ifdef FIFREEZE #define CONFIG_FSFREEZE
@@ -872,6 +873,8 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,GuestDiskAddressList *list =NULL ;bool has_ata = false, has_host = false, has_tgt = false;char *p, *q, *driver =NULL ; +struct udev *udev =NULL ; +struct udev_device *udevice =NULL ; p =strstr (syspath,"/devices/pci" ); if (!p || sscanf(p + 12, "%*x:%*x/%x:%x:%x.%x%n",
@@ -935,6 +938,24 @@ static void build_guest_fsinfo_for_real_device(char const *syspath, list =g_malloc0 (sizeof (*list)); list->value = disk; + udev =udev_new (); + udevice =udev_device_new_from_syspath (udev, syspath); +if (udev ==NULL || udevice ==NULL ) { +g_debug ("failed to query udev" ); + }else { +const char *devnode, *serial; + devnode =udev_device_get_devnode (udevice); +if (devnode !=NULL ) { + disk->dev =g_strdup (devnode); + disk->has_dev = true; + } + serial =udev_device_get_property_value (udevice,"ID_SERIAL" ); +if (serial !=NULL && *serial !=0 ) { + disk->serial =g_strdup (serial); + disk->has_serial = true; + } + } +if (strcmp (driver,"ata_piix" ) ==0 ) {/* a host per ide bus, target*:0:<unit>:0 */ if (!has_host || !has_tgt) {
@@ -1002,6 +1023,8 @@ cleanup:qapi_free_GuestDiskAddressList (list); }g_free (driver); +udev_unref (udev); +udev_device_unref (udevice); } static void build_guest_fsinfo_for_device(char const *devpath,
qga/commands-win32.c +54 -19
120 lines not on this page · raw message
qga/qapi-schema.json +3 -1
@@ -834,13 +834,15 @@ # @bus: bus id # @target: target id # @unit: unit id +# @serial: serial number # # Since: 2.2 ## { 'struct': 'GuestDiskAddress', 'data': {'pci-controller': 'GuestPCIAddress', 'bus-type': 'GuestDiskBusType', - 'bus': 'int', 'target': 'int', 'unit': 'int'} } + 'bus': 'int', 'target': 'int', 'unit': 'int', + '*serial': 'str', '*dev': 'str'} } ## # @GuestFilesystemInfo:
Signature · 1 line
2.18.0
Re: [Qemu-devel] [PATCH 4/4] qga: report serial ID and device node
Quoted 22 lines · Tomáš Golembiovský
On Linux the functionality depends on libudev.
Example from Linux:
{ "name": "dm-2", "mountpoint": "/", ... "disk": [ { "serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493", "dev": "/dev/sda2", ... } ], }Signed-off-by Tomáš Golembiovský <[email protected]>---
+++
b/qga/qapi-schema.json
qga/qapi-schema.json line 837
Quoted 4 lines
qga/qapi-schema.json +1
-0@@ -834,13 +834,15 @@ # @bus: bus id # @target: target id # @unit: unit id +# @serial: serial number
Needs a '(since 3.1)' tag.
Quoted 13 lines
qga/qapi-schema.json +2
-1# # Since: 2.2 ## { 'struct': 'GuestDiskAddress', 'data': {'pci-controller': 'GuestPCIAddress', 'bus-type': 'GuestDiskBusType', - 'bus': 'int', 'target': 'int', 'unit': 'int'} } + 'bus': 'int', 'target': 'int', 'unit': 'int', + '*serial': 'str', '*dev': 'str'} } ## # @GuestFilesystemInfo:
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org