On Tue, 2019-01-29 at 10:17 -0500, Frediano Ziglio wrote: > Can you use the coding style at least for new code? > In particular always use brackets, code is half with and half without Oh, I already pushed after Lukas's ACK. I'll send a follow-up patch fixing some bracket usage. Not sure how I missed so many... > > > > > Add a function to look up an xrandr output for a given device > > display > > id. This uses sysfs and the drm subsystem to lookup information > > about a > > graphics device output. It then compares the drm output name to > > xrandr > > output names to try to match that device output to an xrandr > > output. > > This is necesary for guests that have multiple graphics devices. > > --- > > Makefile.am | 14 ++ > > configure.ac | 1 + > > src/vdagent/device-info.c | 506 > > ++++++++++++++++++++++++++++++++++++++ > > src/vdagent/device-info.h | 30 +++ > > tests/test-device-info.c | 262 ++++++++++++++++++++ > > 5 files changed, 813 insertions(+) > > create mode 100644 src/vdagent/device-info.c > > create mode 100644 src/vdagent/device-info.h > > create mode 100644 tests/test-device-info.c > > > > diff --git a/Makefile.am b/Makefile.am > > index 97b8bf0..c2e9668 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -14,6 +14,7 @@ common_sources = \ > > $(NULL) > > > > src_spice_vdagent_CFLAGS = \ > > + $(DRM_CFLAGS) \ > > $(X_CFLAGS) \ > > $(SPICE_CFLAGS) \ > > $(GLIB2_CFLAGS) \ > > @@ -24,6 +25,7 @@ src_spice_vdagent_CFLAGS = > > \ > > $(NULL) > > > > src_spice_vdagent_LDADD = \ > > + $(DRM_LIBS) \ > > $(X_LIBS) \ > > $(SPICE_LIBS) \ > > $(GLIB2_LIBS) \ > > @@ -37,6 +39,8 @@ src_spice_vdagent_SOURCES = > > \ > > src/vdagent/audio.h \ > > src/vdagent/clipboard.c \ > > src/vdagent/clipboard.h \ > > + src/vdagent/device-info.c \ > > + src/vdagent/device-info.h \ > > src/vdagent/file-xfers.c \ > > src/vdagent/file-xfers.h \ > > src/vdagent/x11-priv.h \ > > @@ -159,3 +163,13 @@ EXTRA_DIST = > > \ > > DISTCHECK_CONFIGURE_FLAGS = \ > > --with-init-script=redhat \ > > $(NULL) > > + > > +tests_test_device_info_LDADD = $(src_spice_vdagent_LDADD) > > +tests_test_device_info_CFLAGS = $(src_spice_vdagent_CFLAGS) > > + > > +check_PROGRAMS = \ > > + tests/test-device-info \ > > + $(NULL) > > + > > +TESTS = $(check_PROGRAMS) > > + > > diff --git a/configure.ac b/configure.ac > > index 7cb44db..55b031e 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -105,6 +105,7 @@ PKG_CHECK_MODULES(X, [xfixes xrandr >= 1.3 > > xinerama x11]) > > PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.13]) > > PKG_CHECK_MODULES(ALSA, [alsa >= 1.0.22]) > > PKG_CHECK_MODULES([DBUS], [dbus-1]) > > +PKG_CHECK_MODULES([DRM], [libdrm]) > > > > if test "$with_session_info" = "auto" || test "$with_session_info" > > = > > "systemd"; then > > PKG_CHECK_MODULES([LIBSYSTEMD_LOGIN], > > diff --git a/src/vdagent/device-info.c b/src/vdagent/device-info.c > > new file mode 100644 > > index 0000000..7c0f615 > > --- /dev/null > > +++ b/src/vdagent/device-info.c > > @@ -0,0 +1,506 @@ > > +/* device-info.c utility function for looking up the xrandr > > output id for a > > + * given device address and display id > > + * > > + * Copyright 2018 Red Hat, Inc. > > + * > > + * Red Hat Authors: > > + * Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > + * > > + * This program is free software: you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as > > published by > > + * the Free Software Foundation, either version 3 of the License, > > or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > License > > + * along with this program. If not, see < > > http://www.gnu.org/licenses/>. > > + */ > > + > > +#include <assert.h> > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <sys/stat.h> > > +#include <syslog.h> > > +#include <xf86drm.h> > > +#include <xf86drmMode.h> > > +#include <unistd.h> > > +#include <X11/extensions/Xrandr.h> > > +#include <glib.h> > > + > > +#include "device-info.h" > > + > > +#define PCI_VENDOR_ID_REDHAT 0x1b36 > > +#define PCI_VENDOR_ID_REDHAT_QUMRANET 0x1af4 // virtio-gpu > > +#define PCI_VENDOR_ID_INTEL 0x8086 > > +#define PCI_VENDOR_ID_NVIDIA 0x10de > > + > > +#define PCI_DEVICE_ID_QXL 0x0100 > > +#define PCI_DEVICE_ID_VIRTIO_GPU 0x1050 > > + > > +typedef struct PciDevice { > > + int domain; > > + uint8_t bus; > > + uint8_t slot; > > + uint8_t function; > > +} PciDevice; > > + > > +typedef struct PciAddress { > > + int domain; > > + GList *devices; /* PciDevice */ > > +} PciAddress; > > + > > +static PciAddress* pci_address_new() > > +{ > > + return g_new0(PciAddress, 1); > > +} > > + > > +static void pci_address_free(PciAddress *addr) > > +{ > > + g_list_free_full(addr->devices, g_free); > > + g_free(addr); > > +} > > + > > + > > +static int read_next_hex_number(const char *input, char delim, > > char > > **endptr) > > +{ > > + assert(input != NULL); > > + assert(endptr != NULL); > > + > > + const char *pos = strchr(input, delim); > > + int n; > > + if (!pos) { > > + *endptr = NULL; > > + return 0; > > + } > > + > > + char *endpos; > > + n = strtol(input, &endpos, 16); > > + > > + // check if we read all characters until the delimiter > > + if (endpos != pos) > > + endpos = NULL; > > + > > + *endptr = endpos; > > + return n; > > +} > > + > > +// the device should be specified in BDF notation (e.g. > > 0000:00:02.0) > > +// see > > https://wiki.xen.org/wiki/Bus:Device.Function_(BDF)_Notation > > +static bool parse_pci_device(const char *bdf, const char *end, > > PciDevice > > *device) > > +{ > > + if (!end) end = strchr(bdf, 0); > > + > > + int endpos = -1; > > + int domain, bus, slot, function; > > + sscanf(bdf, "%x:%x:%x.%x%n", &domain, &bus, &slot, &function, > > &endpos); > > + if (!device || endpos < 0 || bdf + endpos != end) > > + return false; > > + if (domain < 0 || bus < 0 || slot < 0 || function < 0) > > + return false; > > + > > + device->domain = domain; > > + device->bus = bus; > > + device->slot = slot; > > + device->function = function; > > + return true; > > +} > > + > > +// We need to extract the pci address of the device from the sysfs > > entry for > > the device like so: > > +// $ readlink /sys/class/drm/card0 > > +// This should give you a path such as this for cards on the root > > bus: > > +// /sys/devices/pci0000:00/0000:00:02.0/drm/card0 > > +// or something like this if there is a pci bridge: > > +// > > /sys/devices/pci0000:00/0000:00:03.0/0000:01:01.0/0000:02:03.0/virt > > io2/drm/card0 > > +static PciAddress* parse_pci_address_from_sysfs_path(const char* > > addr) > > +{ > > + char *pos = strstr(addr, "/pci"); > > + if (!pos) > > + return NULL; > > + > > + // advance to the numbers in pci0000:00 > > + pos += 4; > > + int domain = read_next_hex_number(pos, ':', &pos); > > + if (!pos) { > > + return NULL; > > + } > > + > > + // not used right now. > > + G_GNUC_UNUSED uint8_t bus = read_next_hex_number(pos + 1, '/', > > &pos); > > + if (!pos) { > > + return NULL; > > + } > > + > > + PciAddress *address = pci_address_new(); > > + address->domain = domain; > > + // now read all of the devices > > + while (pos) { > > + PciDevice *dev = g_new0(PciDevice, 1); > > + char *next = strchr(pos + 1, '/'); > > + if (!parse_pci_device(pos + 1, next, dev)) { > > + g_free(dev); > > + break; > > + } > > + address->devices = g_list_append(address->devices, dev); > > + pos = next; > > + } > > + return address; > > +} > > + > > +// format should be something like pci/$domain/$slot.$fn/$slot.$fn > > +static PciAddress* parse_pci_address_from_spice(char *input) > > +{ > > + static const char prefix[] = "pci/"; > > + if (strncmp(input, prefix, strlen(prefix)) != 0) > > + return NULL; > > + > > + char *pos = input + strlen(prefix); > > + int domain = read_next_hex_number(pos, '/', &pos); > > + if (!pos) { > > + return NULL; > > + } > > + > > + PciAddress *address = pci_address_new(); > > + address->domain = domain; > > + // now read all of the devices > > + for (int n = 0; ; n++) { > > + PciDevice *dev = g_new0(PciDevice, 1); > > + char *next = strchr(pos + 1, '/'); > > + > > + dev->slot = read_next_hex_number(pos + 1, '.', &pos); > > + if (!pos) { > > + g_free(dev); > > + break; > > + } > > + > > + dev->function = strtol(pos + 1, &pos, 16); > > + if (!pos || (next != NULL && next != pos)) { > > + g_free(dev); > > + break; > > + } > > + > > + address->devices = g_list_append(address->devices, dev); > > + pos = next; > > + if (!pos) > > + break; > > + } > > + return address; > > +} > > + > > +static bool compare_addresses(PciAddress *a, PciAddress *b) > > +{ > > + // only check domain, slot, and function > > + if (!(a->domain == b->domain > > + && g_list_length(a->devices) == g_list_length(b- > > >devices))) { > > + return false; > > + } > > + > > + for (GList *la = a->devices, *lb = b->devices; > > + la != NULL; > > + la = la->next, lb = lb->next) { > > + PciDevice *deva = la->data; > > + PciDevice *devb = lb->data; > > + > > + if (deva->slot != devb->slot > > + || deva->function != devb->function) { > > + return false; > > + } > > + } > > + return true; > > +} > > + > > +// Connector type names from xorg modesetting driver > > +static const char * const modesetting_output_names[] = { > > + [DRM_MODE_CONNECTOR_Unknown] = "None" , > > + [DRM_MODE_CONNECTOR_VGA] = "VGA" , > > + [DRM_MODE_CONNECTOR_DVII] = "DVI-I" , > > + [DRM_MODE_CONNECTOR_DVID] = "DVI-D" , > > + [DRM_MODE_CONNECTOR_DVIA] = "DVI-A" , > > + [DRM_MODE_CONNECTOR_Composite] = "Composite" , > > + [DRM_MODE_CONNECTOR_SVIDEO] = "SVIDEO" , > > + [DRM_MODE_CONNECTOR_LVDS] = "LVDS" , > > + [DRM_MODE_CONNECTOR_Component] = "Component" , > > + [DRM_MODE_CONNECTOR_9PinDIN] = "DIN" , > > + [DRM_MODE_CONNECTOR_DisplayPort] = "DP" , > > + [DRM_MODE_CONNECTOR_HDMIA] = "HDMI" , > > + [DRM_MODE_CONNECTOR_HDMIB] = "HDMI-B" , > > + [DRM_MODE_CONNECTOR_TV] = "TV" , > > + [DRM_MODE_CONNECTOR_eDP] = "eDP" , > > + [DRM_MODE_CONNECTOR_VIRTUAL] = "Virtual" , > > + [DRM_MODE_CONNECTOR_DSI] = "DSI" , > > + [DRM_MODE_CONNECTOR_DPI] = "DPI" , > > +}; > > +// Connector type names from qxl driver > > +static const char * const qxl_output_names[] = { > > + [DRM_MODE_CONNECTOR_Unknown] = "None" , > > + [DRM_MODE_CONNECTOR_VGA] = "VGA" , > > + [DRM_MODE_CONNECTOR_DVII] = "DVI" , > > + [DRM_MODE_CONNECTOR_DVID] = "DVI" , > > + [DRM_MODE_CONNECTOR_DVIA] = "DVI" , > > + [DRM_MODE_CONNECTOR_Composite] = "Composite" , > > + [DRM_MODE_CONNECTOR_SVIDEO] = "S-video" , > > + [DRM_MODE_CONNECTOR_LVDS] = "LVDS" , > > + [DRM_MODE_CONNECTOR_Component] = "CTV" , > > + [DRM_MODE_CONNECTOR_9PinDIN] = "DIN" , > > + [DRM_MODE_CONNECTOR_DisplayPort] = "DisplayPort" , > > + [DRM_MODE_CONNECTOR_HDMIA] = "HDMI" , > > + [DRM_MODE_CONNECTOR_HDMIB] = "HDMI" , > > + [DRM_MODE_CONNECTOR_TV] = "TV" , > > + [DRM_MODE_CONNECTOR_eDP] = "eDP" , > > + [DRM_MODE_CONNECTOR_VIRTUAL] = "Virtual" , > > +}; > > + > > + > > +static void drm_conn_name_full(drmModeConnector *conn, > > + const char * const *names, > > + int nnames, char *dest, > > + size_t dlen, bool decrement_id) > > +{ > > + const char *type; > > + > > + if (conn->connector_type < nnames && > > + names[conn->connector_type]) { > > + type = names[conn->connector_type]; > > + } else { > > + type = "unknown"; > > + } > > + > > + uint8_t id = conn->connector_type_id; > > + if (decrement_id) { > > + id--; > > + } > > + snprintf(dest, dlen, "%s-%d", type, id); > > +} > > + > > +static void drm_conn_name_qxl(drmModeConnector *conn, char *dest, > > size_t > > dlen, bool decrement_id) > > +{ > > + return drm_conn_name_full(conn, qxl_output_names, > > + > > sizeof(qxl_output_names)/sizeof(qxl_output_names[0]), > > + dest, dlen, decrement_id); > > +} > > + > > +// NOTE: there are some cases (for example, in a Lenovo T460p > > laptop with > > +// intel graphics when attached to a docking station) where the > > modesetting > > +// driver uses a name such as DP-3-1 instead of DP-4. These > > outputs are not > > +// likely to exist in virtual machines, so they shouldn't matter > > much > > +static void drm_conn_name_modesetting(drmModeConnector *conn, char > > *dest, > > size_t dlen) > > +{ > > + return drm_conn_name_full(conn, modesetting_output_names, > > + > > sizeof(modesetting_output_names)/sizeof(modesetting_output_names[0] > > ), > > + dest, dlen, false); > > +} > > + > > +static bool read_hex_value_from_file(const char *path, int* value) > > +{ > > + if (value == NULL || path == NULL) > > + return false; > > + > > + FILE *f = fopen(path, "r"); > > + if (f == NULL) > > + return false; > > + > > + int endpos = -1; > > + bool result = (fscanf(f, "%x\n%n", value, &endpos) > 0 && > > endpos >= 0); > > + > > + fclose(f); > > + return result; > > +} > > + > > +// returns a path to a drm device found at the given PCI Address. > > Returned > > +// string must be freed by caller. > > +static char* find_device_at_pci_address(PciAddress *pci_addr, int > > *vendor_id, int *device_id) > > +{ > > + g_return_val_if_fail(pci_addr != NULL, NULL); > > + g_return_val_if_fail(device_id != NULL, NULL); > > + g_return_val_if_fail(vendor_id != NULL, NULL); > > + // Look for a device that matches the PCI address parsed > > above. Loop > > + // through the list of cards reported by the DRM subsytem > > + for (int i = 0; i < 10; ++i) { > > + char dev_path[64]; > > + struct stat buf; > > + > > + // device node for the card is needed to access libdrm > > functionality > > + snprintf(dev_path, sizeof(dev_path), DRM_DEV_NAME, > > DRM_DIR_NAME, i); > > + if (stat(dev_path, &buf) != 0) { > > + // no card exists, exit loop > > + syslog(LOG_DEBUG, "card%i not found while listing DRM > > devices.", > > i); > > + break; > > + } > > + > > + // the sysfs directory for the card will allow us to > > determine the > > + // pci address for the device > > + char sys_path[64]; > > + snprintf(sys_path, sizeof(sys_path), > > "/sys/class/drm/card%d", i); > > + > > + // the file /sys/class/drm/card0 is a symlink to a file > > that > > + // specifies the device's address. It usually points to > > something > > + // like /sys/devices/pci0000:00/0000:00:02.0/drm/card0 > > + char device_link[PATH_MAX]; > > + if (realpath(sys_path, device_link) == NULL) { > > + syslog(LOG_WARNING, "Failed to get the real path of > > %s", > > sys_path); > > + break; > > + } > > + syslog(LOG_DEBUG, "Device %s is at %s", dev_path, > > device_link); > > + > > + PciAddress *drm_pci_addr = > > parse_pci_address_from_sysfs_path(device_link); > > + if (!drm_pci_addr) { > > + syslog(LOG_WARNING, "Can't determine pci address from > > '%s'", > > device_link); > > + continue; > > + } > > + > > + if (!compare_addresses(pci_addr, drm_pci_addr)) { > > + pci_address_free(drm_pci_addr); > > + continue; > > + } > > + pci_address_free(drm_pci_addr); > > + char id_path[150]; > > + snprintf(id_path, sizeof(id_path), "%s/device/vendor", > > sys_path); > > + if (!read_hex_value_from_file(id_path, vendor_id)) { > > + syslog(LOG_WARNING, "Unable to read vendor ID of card: > > %s", > > strerror(errno)); > > + } > > + snprintf(id_path, sizeof(id_path), "%s/device/device", > > sys_path); > > + if (!read_hex_value_from_file(id_path, device_id)) { > > + syslog(LOG_WARNING, "Unable to read device ID of card: > > %s", > > strerror(errno)); > > + } > > + > > + syslog(LOG_DEBUG, "Found card '%s' with Vendor ID %#x, > > Device ID > > %#x", > > + device_link, *device_id, *vendor_id); > > + return g_strdup(dev_path); > > + } > > + return NULL; > > +} > > + > > +// PCI address should be in the following format: > > +// pci/$domain/$slot.$fn/$slot.$fn > > +bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo > > *device_info, > > + Display *xdisplay, > > + XRRScreenResources > > *xres, > > + RROutput *output_id) > > +{ > > + PciAddress *user_pci_addr = > > parse_pci_address_from_spice((char*)device_info->device_address); > > + if (!user_pci_addr) { > > + syslog(LOG_WARNING, > > + "Couldn't parse PCI address '%s'. " > > + "Address should be the form > > 'pci/$domain/$slot.$fn/$slot.fn...", > > + device_info->device_address); > > + return false; > > + } > > + > > + int vendor_id = 0; > > + int device_id = 0; > > + char *dev_path = find_device_at_pci_address(user_pci_addr, > > &vendor_id, > > &device_id); > > + pci_address_free(user_pci_addr); > > + > > + int drm_fd = open(dev_path, O_RDWR); > > + if (drm_fd < 0) { > > + syslog(LOG_WARNING, "Unable to open file %s", dev_path); > > + return false; > > + } > > + > > + drmModeResPtr res = drmModeGetResources(drm_fd); > > + if (res) { > > + // find the drm output that is equal to device_display_id > > + if (device_info->device_display_id >= res- > > >count_connectors) { > > + syslog(LOG_WARNING, > > + "Specified display id %i is higher than the > > maximum > > display id " > > + "provided by this device (%i)", > > + device_info->device_display_id, res- > > >count_connectors - > > 1); > > + close(drm_fd); > > + return false; > > + } > > + > > + drmModeConnectorPtr conn = > > + drmModeGetConnector(drm_fd, > > res->connectors[device_info->device_display_id]); > > + drmModeFreeResources(res); > > + res = NULL; > > + close(drm_fd); > > + > > + if (conn == NULL) { > > + syslog(LOG_WARNING, "Unable to get drm connector for > > display id > > %i", > > + device_info->device_display_id); > > + return false; > > + } > > + > > + bool decrement_name = false; > > + if (vendor_id == PCI_VENDOR_ID_REDHAT && device_id == > > PCI_DEVICE_ID_QXL) { > > + // Older QXL drivers numbered their outputs starting > > with > > + // 0. This contrasts with most drivers who start > > numbering > > + // outputs with 1. In this case, the expected drm > > connector > > + // name will need to be decremented before comparing > > to the > > + // xrandr output name > > + for (int i = 0; i < xres->noutput; ++i) { > > + XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, > > xres, > > xres->outputs[i]); > > + if (!oinfo) { > > + syslog(LOG_WARNING, "Unable to lookup XRandr > > output info > > for output %li", > > + xres->outputs[i]); > > + return false; > > + } > > + if (strcmp(oinfo->name, "Virtual-0") == 0) { > > + decrement_name = true; > > + XRRFreeOutputInfo(oinfo); > > + break; > > + } > > + XRRFreeOutputInfo(oinfo); > > + } > > + } > > + // Compare the name of the xrandr output against what we > > would > > + // expect based on the drm connection type. The xrandr > > names > > + // are driver-specific, so we need to special-case some > > + // drivers. Most hardware these days uses the > > 'modesetting' > > + // driver, but the QXL device uses its own driver which > > has > > + // different naming conventions > > + char expected_name[100]; > > + if (vendor_id == PCI_VENDOR_ID_REDHAT && device_id == > > PCI_DEVICE_ID_QXL) { > > + drm_conn_name_qxl(conn, expected_name, > > sizeof(expected_name), > > decrement_name); > > + } else { > > + drm_conn_name_modesetting(conn, expected_name, > > sizeof(expected_name)); > > + } > > + > > + // Loop through xrandr outputs and check whether the > > xrandr > > + // output name matches the drm connector name > > + for (int i = 0; i < xres->noutput; ++i) { > > + int oid = xres->outputs[i]; > > + XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, > > xres, oid); > > + if (!oinfo) { > > + syslog(LOG_WARNING, "Unable to lookup XRandr > > output info for > > output %i", oid); > > + return false; > > + } > > + > > + if (strcmp(oinfo->name, expected_name) == 0) { > > + *output_id = oid; > > + syslog(LOG_DEBUG, "Found matching X Output: > > name=%s id=%i", > > + oinfo->name, (int)oid); > > + XRRFreeOutputInfo(oinfo); > > + return true; > > + } > > + XRRFreeOutputInfo(oinfo); > > + } > > + drmModeFreeConnector(conn); > > + } else { > > + close(drm_fd); > > + syslog(LOG_WARNING, > > + "Unable to get DRM resources for card %s. " > > + "Falling back to using xrandr output index.", > > + dev_path); > > + // This is probably a proprietary driver (e.g. Nvidia) > > that does > > + // not provide outputs via drm, so the only thing we can > > do is just > > + // assume that it is the only device assigned to X, and > > use the > > + // xrandr output order to determine the proper display. > > + if (device_info->device_display_id >= xres->noutput) { > > + syslog(LOG_WARNING, "The device display id %i does not > > exist", > > + device_info->device_display_id); > > + return false; > > + } > > + *output_id = xres->outputs[device_info- > > >device_display_id]; > > + return true; > > + } > > + > > + syslog(LOG_WARNING, "Couldn't find an XRandr output for the > > specified > > device"); > > + return false; > > +} > > diff --git a/src/vdagent/device-info.h b/src/vdagent/device-info.h > > new file mode 100644 > > index 0000000..8646cc5 > > --- /dev/null > > +++ b/src/vdagent/device-info.h > > @@ -0,0 +1,30 @@ > > +/* device-info.c utility function for looking up the xrandr > > output id for a > > + * given device address and display id > > + * > > + * Copyright 2018 Red Hat, Inc. > > + * > > + * Red Hat Authors: > > + * Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > + * > > + * This program is free software: you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as > > published by > > + * the Free Software Foundation, either version 3 of the License, > > or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > License > > + * along with this program. If not, see < > > http://www.gnu.org/licenses/>. > > + */ > > +#include <spice/vd_agent.h> > > +#include <X11/extensions/Xrandr.h> > > +#include <stdbool.h> > > + > > +struct vdagent_x11; > > +bool lookup_xrandr_output_for_device_info(VDAgentDeviceDisplayInfo > > *device_info, > > + Display *xdisplay, > > + XRRScreenResources > > *xres, > > + RROutput *output_id); > > diff --git a/tests/test-device-info.c b/tests/test-device-info.c > > new file mode 100644 > > index 0000000..5964313 > > --- /dev/null > > +++ b/tests/test-device-info.c > > @@ -0,0 +1,262 @@ > > +/* test-device-info.c tests to see whether the functionality for > > converting > > a > > + * device address to a xrandr output are working properly > > + * > > + * Copyright 2018 Red Hat, Inc. > > + * > > + * Red Hat Authors: > > + * Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > + * > > + * This program is free software: you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as > > published by > > + * the Free Software Foundation, either version 3 of the License, > > or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > License > > + * along with this program. If not, see < > > http://www.gnu.org/licenses/>. > > + */ > > + > > +// just include the source file for testing > > +#include "vdagent/device-info.c" > > + > > +#define assert_device(dev, domain_, bus_, slot_, function_) \ > > +{ \ > > + PciDevice* dev_ = (dev); \ > > + assert(dev_ != NULL); \ > > + assert(dev_->domain == domain_); \ > > + assert(dev_->bus == bus_); \ > > + assert(dev_->slot == slot_); \ > > + assert(dev_->function == function_); \ > > +} > > + > > +static void test_compare_addresses() > > +{ > > + { > > + PciDevice da1 = {1, 0, 3, 0}; > > + PciDevice da2 = {1, 1, 1, 0}; > > + PciDevice da3 = {1, 2, 3, 0}; > > + PciAddress a1 = {1, NULL}; > > + a1.domain = 0; > > + a1.devices = g_list_append(a1.devices, &da1); > > + a1.devices = g_list_append(a1.devices, &da2); > > + a1.devices = g_list_append(a1.devices, &da3); > > + > > + PciDevice db1 = {1, 0, 3, 0}; > > + PciDevice db2 = {1, 1, 1, 0}; > > + PciDevice db3 = {1, 2, 3, 0}; > > + PciAddress a2 = {1, NULL}; > > + a2.domain = 0; > > + a2.devices = g_list_append(a2.devices, &db1); > > + a2.devices = g_list_append(a2.devices, &db2); > > + a2.devices = g_list_append(a2.devices, &db3); > > + > > + assert(compare_addresses(&a1, &a2)); > > + } > > + { > > + PciDevice da1 = {1, 0, 3, 0}; > > + PciDevice da2 = {1, 1, 1, 0}; > > + PciDevice da3 = {1, 2, 3, 0}; > > + PciAddress a1 = {1, NULL}; > > + a1.domain = 0; > > + a1.devices = g_list_append(a1.devices, &da1); > > + a1.devices = g_list_append(a1.devices, &da2); > > + a1.devices = g_list_append(a1.devices, &da3); > > + > > + // a 'spice' format PCI address will not provide domain or > > bus for > > each > > + // device, only slot and function. So first two numbers > > for each > > device > > + // will always be set to 0 > > + PciDevice db1 = {0, 0, 3, 0}; > > + PciDevice db2 = {0, 0, 1, 0}; > > + PciDevice db3 = {0, 0, 3, 0}; > > + PciAddress a2 = {1, NULL}; > > + a2.domain = 0; > > + a2.devices = g_list_append(a2.devices, &db1); > > + a2.devices = g_list_append(a2.devices, &db2); > > + a2.devices = g_list_append(a2.devices, &db3); > > + > > + assert(compare_addresses(&a1, &a2)); > > + } > > + // different number of devices > > + { > > + PciDevice da1 = {0, 0, 3, 0}; > > + PciDevice da2 = {0, 1, 1, 0}; > > + PciDevice da3 = {0, 2, 3, 0}; > > + PciAddress a1 = {0, NULL}; > > + a1.domain = 0; > > + a1.devices = g_list_append(a1.devices, &da1); > > + a1.devices = g_list_append(a1.devices, &da2); > > + a1.devices = g_list_append(a1.devices, &da3); > > + > > + PciDevice db1 = {0, 0, 3, 0}; > > + PciDevice db2 = {0, 1, 1, 0}; > > + PciAddress a2 = {0, NULL}; > > + a2.domain = 0; > > + a2.devices = g_list_append(a2.devices, &db1); > > + a2.devices = g_list_append(a2.devices, &db2); > > + > > + assert(!compare_addresses(&a1, &a2)); > > + } > > + // mismatched function > > + { > > + PciDevice da1 = {0, 0, 2, 0}; > > + PciAddress a1 = {0, NULL}; > > + a1.domain = 0; > > + a1.devices = g_list_append(a1.devices, &da1); > > + > > + PciDevice db1 = {0, 0, 2, 1}; > > + PciAddress a2 = {0, NULL}; > > + a2.domain = 0; > > + a2.devices = g_list_append(a2.devices, &db1); > > + > > + assert(!compare_addresses(&a1, &a2)); > > + } > > + // mismatched slot > > + { > > + PciDevice da1 = {0, 0, 2, 0}; > > + PciAddress a1 = {0, NULL}; > > + a1.domain = 0; > > + a1.devices = g_list_append(a1.devices, &da1); > > + > > + PciDevice db1 = {0, 0, 1, 0}; > > + PciAddress a2 = {0, NULL}; > > + a2.domain = 0; > > + a2.devices = g_list_append(a2.devices, &db1); > > + > > + assert(!compare_addresses(&a1, &a2)); > > + } > > + // mismatched domain > > + { > > + PciDevice da1 = {0, 0, 2, 0}; > > + PciAddress a1 = {0, NULL}; > > + a1.domain = 1; > > + a1.devices = g_list_append(a1.devices, &da1); > > + > > + PciDevice db1 = {0, 0, 2, 0}; > > + PciAddress a2 = {0, NULL}; > > + a2.domain = 0; > > + a2.devices = g_list_append(a2.devices, &db1); > > + > > + assert(!compare_addresses(&a1, &a2)); > > + } > > +} > > + > > +static void test_spice_parsing() > > +{ > > + PciAddress *addr = > > parse_pci_address_from_spice("pci/0000/02.0"); > > + assert(addr != NULL); > > + assert(addr->domain == 0); > > + assert(g_list_length(addr->devices) == 1); > > + assert_device(addr->devices->data, 0, 0, 2, 0); > > + pci_address_free(addr); > > + > > + addr = parse_pci_address_from_spice("pci/ffff/ff.f"); > > + assert(addr != NULL); > > + assert(addr->domain == 65535); > > + assert(g_list_length(addr->devices) == 1); > > + assert_device(addr->devices->data, 0, 0, 255, 15); > > + pci_address_free(addr); > > + > > + addr = parse_pci_address_from_spice("pci/0000/02.1/03.0"); > > + assert(addr != NULL); > > + assert(addr->domain == 0); > > + assert(g_list_length(addr->devices) == 2); > > + assert_device(addr->devices->data, 0, 0, 2, 1); > > + assert_device(addr->devices->next->data, 0, 0, 3, 0); > > + pci_address_free(addr); > > + > > + addr = > > parse_pci_address_from_spice("pci/000a/01.0/02.1/03.0"); > > + assert(addr != NULL); > > + assert(addr->domain == 10); > > + assert(g_list_length(addr->devices) == 3); > > + assert_device(addr->devices->data, 0, 0, 1, 0); > > + assert_device(addr->devices->next->data, 0, 0, 2, 1); > > + assert_device(addr->devices->next->next->data, 0, 0, 3, 0); > > + pci_address_free(addr); > > + > > + addr = parse_pci_address_from_spice("pcx/0000/02.1/03.0"); > > + assert(addr == NULL); > > + > > + addr = parse_pci_address_from_spice("0000/02.0"); > > + assert(addr == NULL); > > + > > + addr = parse_pci_address_from_spice("0000/02.1/03.0"); > > + assert(addr == NULL); > > +} > > + > > +static void test_sysfs_parsing() > > +{ > > + PciAddress *addr = > > parse_pci_address_from_sysfs_path("../../devices/pci0000:00/0000:00 > > :02.0/drm/card0"); > > + assert(addr != NULL); > > + assert(addr->domain == 0); > > + assert(g_list_length(addr->devices) == 1); > > + assert_device(addr->devices->data, 0, 0, 2, 0); > > + pci_address_free(addr); > > + > > + addr = > > parse_pci_address_from_sysfs_path("../../devices/pciffff:ff/ffff:ff > > :ff.f/drm/card0"); > > + assert(addr != NULL); > > + assert(addr->domain == 65535); > > + assert(g_list_length(addr->devices) == 1); > > + assert_device(addr->devices->data, 65535, 255, 255, 15); > > + pci_address_free(addr); > > + > > + addr = > > parse_pci_address_from_sysfs_path("../../devices/pci0000:00/0000:00 > > :03.0/0000:01:01.0/0000:02:03.0/virtio2/drm/card0"); > > + assert(addr != NULL); > > + assert(addr->domain == 0); > > + assert(g_list_length(addr->devices) == 3); > > + assert_device(addr->devices->data, 0, 0, 3, 0); > > + assert_device(addr->devices->next->data, 0, 1, 1, 0); > > + assert_device(addr->devices->next->next->data, 0, 2, 3, 0); > > + pci_address_free(addr); > > +} > > + > > +// verify that we parse the BDF notation correctly > > +static bool test_bdf(const char* string, int domain, uint8_t bus, > > uint8_t > > slot, uint8_t function) > > +{ > > + PciDevice pci_dev; > > + return (parse_pci_device(string, NULL, &pci_dev) > > + && (pci_dev.domain == domain) > > + && (pci_dev.bus == bus) > > + && (pci_dev.slot == slot) > > + && (pci_dev.function == function)); > > +} > > + > > +static void test_bdf_parsing() > > +{ > > + // valid input > > + assert(test_bdf("0000:00:02.1", 0, 0, 2, 1)); > > + assert(test_bdf("00:00:02.1", 0, 0, 2, 1)); > > + assert(test_bdf("0000:00:03.0", 0, 0, 3, 0)); > > + assert(test_bdf("0000:00:1d.1", 0, 0, 29, 1)); > > + assert(test_bdf("0000:09:02.1", 0, 9, 2, 1)); > > + assert(test_bdf("0000:1d:02.1", 0, 29, 2, 1)); > > + assert(test_bdf("0000:00:02.d", 0, 0, 2, 13)); > > + assert(test_bdf("000f:00:02.d", 15, 0, 2, 13)); > > + assert(test_bdf("000f:00:02.d", 15, 0, 2, 13)); > > + assert(test_bdf("000f:00:02.d", 15, 0, 2, 13)); > > + assert(test_bdf("000f:00:02.d", 15, 0, 2, 13)); > > + assert(test_bdf("ffff:ff:ff.f", 65535, 255, 255, 15)); > > + assert(test_bdf("0:0:2.1", 0, 0, 2, 1)); > > + > > + // invalid input > > + assert(!test_bdf("0000:00:02:0", 0, 0, 2, 0)); > > + assert(!test_bdf("-0001:00:02.1", -1, 0, 2, 1)); > > + assert(!test_bdf("0000.00.02.0", 0, 0, 2, 0)); > > + assert(!test_bdf("000f:00:02", 15, 0, 2, 0)); > > + assert(!test_bdf("000f:00", 15, 0, 0, 0)); > > + assert(!test_bdf("000f", 15, 0, 0, 0)); > > + assert(!test_bdf("random string", 0, 0, 0, 0)); > > + assert(!test_bdf("12345", 12345, 0, 0, 0)); > > +} > > + > > +int main(int argc, char **argv) > > +{ > > + test_bdf_parsing(); > > + test_sysfs_parsing(); > > + test_spice_parsing(); > > + test_compare_addresses(); > > +} > > + > > -- > > 2.17.2 > > > > _______________________________________________ > > Spice-devel mailing list > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/spice-devel > > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel