On Wed, 23 Dec 2015, Cao jin wrote:
> Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
> ---
>
> Since the callchain is pretty deep & error path is very much too, so I made
> the
> patch based on the principal: catch/report the most necessary error msg with
> smallest modification.(So you can see I don`t change some functions to void,
> despite they coule be)
Thanks Cao.
For consistency with the other functions, I think it would be better to
change all functions to return void or none.
Also it might be nice to split the patch in a series.
The patch as is fails to build:
qemu/hw/xen/xen_pt_config_init.c: In function ‘xen_pt_config_init’:
qemu/hw/xen/xen_pt_config_init.c:2061:42: error: ‘rc’ may be used uninitialized
in this func
> hw/xen/xen-host-pci-device.c | 79
> +++++++++++++++++++++++++++-----------------
> hw/xen/xen-host-pci-device.h | 5 +--
> hw/xen/xen_pt.c | 67 +++++++++++++++++++------------------
> hw/xen/xen_pt.h | 5 +--
> hw/xen/xen_pt_config_init.c | 47 +++++++++++++-------------
> hw/xen/xen_pt_graphics.c | 6 ++--
> 6 files changed, 118 insertions(+), 91 deletions(-)
>
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 7d8a023..1ab6d97 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -43,13 +43,14 @@ static int xen_host_pci_sysfs_path(const XenHostPCIDevice
> *d,
> /* The output is truncated, or some other error was encountered */
> return -ENODEV;
> }
> +
> return 0;
> }
I would prefer to keep stylistic changes separate, especially the ones
in functions which would be otherwise left unmodified. Maybe you could
move them to a separate patch?
> /* This size should be enough to read the first 7 lines of a resource file */
> #define XEN_HOST_PCI_RESOURCE_BUFFER_SIZE 400
> -static int xen_host_pci_get_resource(XenHostPCIDevice *d)
> +static void xen_host_pci_get_resource(XenHostPCIDevice *d, Error **errp)
> {
> int i, rc, fd;
> char path[PATH_MAX];
> @@ -60,23 +61,24 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
>
> rc = xen_host_pci_sysfs_path(d, "resource", path, sizeof (path));
> if (rc) {
> - return rc;
> + error_setg_errno(errp, errno, "snprintf err");
> + return;
> }
> +
> fd = open(path, O_RDONLY);
> if (fd == -1) {
> - XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path,
> strerror(errno));
> - return -errno;
> + error_setg_errno(errp, errno, "open err");
> + return;
> }
>
> do {
> rc = read(fd, &buf, sizeof (buf) - 1);
> if (rc < 0 && errno != EINTR) {
> - rc = -errno;
> + error_setg_errno(errp, errno, "read err");
> goto out;
> }
> } while (rc < 0);
> buf[rc] = 0;
> - rc = 0;
>
> s = buf;
> for (i = 0; i < PCI_NUM_REGIONS; i++) {
> @@ -129,20 +131,20 @@ static int xen_host_pci_get_resource(XenHostPCIDevice
> *d)
> d->rom.bus_flags = flags & IORESOURCE_BITS;
> }
> }
> +
> if (i != PCI_NUM_REGIONS) {
> /* Invalid format or input to short */
> - rc = -ENODEV;
> + error_setg(errp, "Invalid format or input to short");
^too short
> }
>
> out:
> close(fd);
> - return rc;
> }
>
> /* This size should be enough to read a long from a file */
> #define XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE 22
> static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
> - unsigned int *pvalue, int base)
> + unsigned int *pvalue, int base, Error
> **errp)
As mentioned above, I would prefer if you could change this function to
return void too. Otherwise keep the return int everywhere.
> {
> char path[PATH_MAX];
> char buf[XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE];
> @@ -152,30 +154,38 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d,
> const char *name,
>
> rc = xen_host_pci_sysfs_path(d, name, path, sizeof (path));
> if (rc) {
> + error_setg_errno(errp, errno, "snprintf err");
> return rc;
> }
> +
> fd = open(path, O_RDONLY);
> if (fd == -1) {
> - XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path,
> strerror(errno));
> + error_setg_errno(errp, errno, "open err");
would it be possible to keep the path in the error message?
> return -errno;
> }
> +
> do {
> rc = read(fd, &buf, sizeof (buf) - 1);
> if (rc < 0 && errno != EINTR) {
> + error_setg_errno(errp, errno, "read err");
> rc = -errno;
> goto out;
> }
> } while (rc < 0);
> +
> buf[rc] = 0;
> value = strtol(buf, &endptr, base);
> if (endptr == buf || *endptr != '\n') {
> + error_setg(errp, "format invalid");
> rc = -1;
> } else if ((value == LONG_MIN || value == LONG_MAX) && errno == ERANGE) {
> + error_setg_errno(errp, errno, "strtol err");
> rc = -errno;
> } else {
> rc = 0;
> *pvalue = value;
> }
> +
> out:
> close(fd);
> return rc;
> @@ -183,16 +193,18 @@ out:
>
> static inline int xen_host_pci_get_hex_value(XenHostPCIDevice *d,
> const char *name,
> - unsigned int *pvalue)
> + unsigned int *pvalue,
> + Error **errp)
same here about returning void
> {
> - return xen_host_pci_get_value(d, name, pvalue, 16);
> + return xen_host_pci_get_value(d, name, pvalue, 16, errp);
> }
>
> static inline int xen_host_pci_get_dec_value(XenHostPCIDevice *d,
> const char *name,
> - unsigned int *pvalue)
> + unsigned int *pvalue,
> + Error **errp)
same here
> {
> - return xen_host_pci_get_value(d, name, pvalue, 10);
> + return xen_host_pci_get_value(d, name, pvalue, 10, errp);
> }
>
> static bool xen_host_pci_dev_is_virtfn(XenHostPCIDevice *d)
> @@ -206,20 +218,21 @@ static bool xen_host_pci_dev_is_virtfn(XenHostPCIDevice
> *d)
> return !stat(path, &buf);
> }
>
> -static int xen_host_pci_config_open(XenHostPCIDevice *d)
> +static void xen_host_pci_config_open(XenHostPCIDevice *d, Error **errp)
> {
> char path[PATH_MAX];
> int rc;
>
> rc = xen_host_pci_sysfs_path(d, "config", path, sizeof (path));
> if (rc) {
> - return rc;
> + error_setg_errno(errp, errno, "snprintf err");
> + return;
> }
> +
> d->config_fd = open(path, O_RDWR);
> if (d->config_fd < 0) {
> - return -errno;
> + error_setg_errno(errp, errno, "open err");
> }
> - return 0;
> }
>
> static int xen_host_pci_config_read(XenHostPCIDevice *d,
> @@ -341,8 +354,9 @@ int xen_host_pci_find_ext_cap_offset(XenHostPCIDevice *d,
> uint32_t cap)
> return -1;
> }
>
> -int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> - uint8_t bus, uint8_t dev, uint8_t func)
> +void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> + uint8_t bus, uint8_t dev, uint8_t func,
> + Error **errp)
> {
> unsigned int v;
> int rc = 0;
> @@ -353,43 +367,48 @@ int xen_host_pci_device_get(XenHostPCIDevice *d,
> uint16_t domain,
> d->dev = dev;
> d->func = func;
>
> - rc = xen_host_pci_config_open(d);
> - if (rc) {
> + xen_host_pci_config_open(d, errp);
> + if (*errp) {
> goto error;
> }
> - rc = xen_host_pci_get_resource(d);
> - if (rc) {
> +
> + xen_host_pci_get_resource(d, errp);
> + if (*errp) {
> goto error;
> }
> - rc = xen_host_pci_get_hex_value(d, "vendor", &v);
> +
> + rc = xen_host_pci_get_hex_value(d, "vendor", &v, errp);
> if (rc) {
please stick to the same strategy to check for returned errors
> goto error;
> }
> d->vendor_id = v;
> - rc = xen_host_pci_get_hex_value(d, "device", &v);
> +
> + rc = xen_host_pci_get_hex_value(d, "device", &v, errp);
> if (rc) {
> goto error;
> }
> d->device_id = v;
> - rc = xen_host_pci_get_dec_value(d, "irq", &v);
> +
> + rc = xen_host_pci_get_dec_value(d, "irq", &v, errp);
> if (rc) {
> goto error;
> }
> d->irq = v;
> - rc = xen_host_pci_get_hex_value(d, "class", &v);
> +
> + rc = xen_host_pci_get_hex_value(d, "class", &v, errp);
> if (rc) {
> goto error;
> }
> d->class_code = v;
> +
> d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
>
> - return 0;
> + return;
> error:
> if (d->config_fd >= 0) {
> close(d->config_fd);
> d->config_fd = -1;
> }
> - return rc;
> }
>
> bool xen_host_pci_device_closed(XenHostPCIDevice *d)
> diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
> index 3d44e04..42b9138 100644
> --- a/hw/xen/xen-host-pci-device.h
> +++ b/hw/xen/xen-host-pci-device.h
> @@ -36,8 +36,9 @@ typedef struct XenHostPCIDevice {
> int config_fd;
> } XenHostPCIDevice;
>
> -int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> - uint8_t bus, uint8_t dev, uint8_t func);
> +void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> + uint8_t bus, uint8_t dev, uint8_t func,
> + Error **errp);
> void xen_host_pci_device_put(XenHostPCIDevice *pci_dev);
> bool xen_host_pci_device_closed(XenHostPCIDevice *d);
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index aa96288..5f4cbb7 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -760,10 +760,10 @@ static void xen_pt_destroy(PCIDevice *d) {
> }
> /* init */
>
> -static int xen_pt_initfn(PCIDevice *d)
> +static void xen_pt_realize(PCIDevice *d, Error **errp)
> {
> XenPCIPassthroughState *s = XEN_PT_DEVICE(d);
> - int rc = 0;
> + int i, rc = 0;
> uint8_t machine_irq = 0, scratch;
> uint16_t cmd = 0;
> int pirq = XEN_PT_UNASSIGNED_PIRQ;
> @@ -774,12 +774,11 @@ static int xen_pt_initfn(PCIDevice *d)
> s->hostaddr.bus, s->hostaddr.slot, s->hostaddr.function,
> s->dev.devfn);
>
> - rc = xen_host_pci_device_get(&s->real_device,
> - s->hostaddr.domain, s->hostaddr.bus,
> - s->hostaddr.slot, s->hostaddr.function);
> - if (rc) {
> - XEN_PT_ERR(d, "Failed to \"open\" the real pci device. rc: %i\n",
> rc);
> - return -1;
> + xen_host_pci_device_get(&s->real_device,
> + s->hostaddr.domain, s->hostaddr.bus,
> + s->hostaddr.slot, s->hostaddr.function, errp);
> + if (*errp) {
> + return;
> }
>
> s->is_virtfn = s->real_device.is_virtfn;
> @@ -799,16 +798,17 @@ static int xen_pt_initfn(PCIDevice *d)
> if ((s->real_device.domain == 0) && (s->real_device.bus == 0) &&
> (s->real_device.dev == 2) && (s->real_device.func == 0)) {
> if (!is_igd_vga_passthrough(&s->real_device)) {
> - XEN_PT_ERR(d, "Need to enable igd-passthru if you're trying"
> - " to passthrough IGD GFX.\n");
> + error_setg(errp, "Need to enable igd-passthru if you're trying"
> + " to passthrough IGD GFX.");
> xen_host_pci_device_put(&s->real_device);
> - return -1;
> + return;
> }
>
> - if (xen_pt_setup_vga(s, &s->real_device) < 0) {
> - XEN_PT_ERR(d, "Setup VGA BIOS of passthrough GFX failed!\n");
> + if (xen_pt_setup_vga(s, &s->real_device, errp) < 0) {
> + error_append_hint(errp, "Setup VGA BIOS of passthrough"
> + " GFX failed!");
> xen_host_pci_device_put(&s->real_device);
> - return -1;
> + return;
> }
>
> /* Register ISA bridge for passthrough GFX. */
> @@ -819,29 +819,28 @@ static int xen_pt_initfn(PCIDevice *d)
> xen_pt_register_regions(s, &cmd);
>
> /* reinitialize each config register to be emulated */
> - rc = xen_pt_config_init(s);
> - if (rc) {
> - XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
> + xen_pt_config_init(s, errp);
> + if (*errp) {
> + error_append_hint(errp, "PCI Config space initialisation failed.");
> goto err_out;
> }
>
> /* Bind interrupt */
> rc = xen_host_pci_get_byte(&s->real_device, PCI_INTERRUPT_PIN, &scratch);
> if (rc) {
> - XEN_PT_ERR(d, "Failed to read PCI_INTERRUPT_PIN! (rc:%d)\n", rc);
> + error_setg_errno(errp, errno, "Failed to read PCI_INTERRUPT_PIN!");
> goto err_out;
> }
> if (!scratch) {
> - XEN_PT_LOG(d, "no pin interrupt\n");
> + error_setg(errp, "no pin interrupt");
> goto out;
> }
>
> machine_irq = s->real_device.irq;
> rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
> -
> if (rc < 0) {
> - XEN_PT_ERR(d, "Mapping machine irq %u to pirq %i failed, (err:
> %d)\n",
> - machine_irq, pirq, errno);
> + error_setg_errno(errp, errno, "Mapping machine irq %u to"
> + " pirq %i failed", machine_irq, pirq);
>
> /* Disable PCI intx assertion (turn on bit10 of devctl) */
> cmd |= PCI_COMMAND_INTX_DISABLE;
> @@ -862,8 +861,8 @@ static int xen_pt_initfn(PCIDevice *d)
> PCI_SLOT(d->devfn),
> e_intx);
> if (rc < 0) {
> - XEN_PT_ERR(d, "Binding of interrupt %i failed! (err: %d)\n",
> - e_intx, errno);
> + error_setg_errno(errp, errno, "Binding of interrupt %i failed!",
> + e_intx);
>
> /* Disable PCI intx assertion (turn on bit10 of devctl) */
> cmd |= PCI_COMMAND_INTX_DISABLE;
> @@ -871,8 +870,8 @@ static int xen_pt_initfn(PCIDevice *d)
>
> if (xen_pt_mapped_machine_irq[machine_irq] == 0) {
> if (xc_physdev_unmap_pirq(xen_xc, xen_domid, machine_irq)) {
> - XEN_PT_ERR(d, "Unmapping of machine interrupt %i failed!"
> - " (err: %d)\n", machine_irq, errno);
> + error_setg_errno(errp, errno, "Unmapping of machine"
> + " interrupt %i failed!", machine_irq);
> }
> }
> s->machine_irq = 0;
> @@ -885,14 +884,14 @@ out:
>
> rc = xen_host_pci_get_word(&s->real_device, PCI_COMMAND, &val);
> if (rc) {
> - XEN_PT_ERR(d, "Failed to read PCI_COMMAND! (rc: %d)\n", rc);
> + error_setg_errno(errp, errno, "Failed to read PCI_COMMAND!");
> goto err_out;
> } else {
> val |= cmd;
> rc = xen_host_pci_set_word(&s->real_device, PCI_COMMAND, val);
> if (rc) {
> - XEN_PT_ERR(d, "Failed to write PCI_COMMAND val=0x%x!(rc:
> %d)\n",
> - val, rc);
> + error_setg_errno(errp, errno, "Failed to write PCI_COMMAND"
> + " val=0x%x!", val);
> goto err_out;
> }
> }
> @@ -905,12 +904,16 @@ out:
> "Real physical device %02x:%02x.%d registered
> successfully!\n",
> s->hostaddr.bus, s->hostaddr.slot, s->hostaddr.function);
>
> - return 0;
> + return;
>
> err_out:
> + for (i = 0; i < PCI_ROM_SLOT; i++) {
> + object_unparent(OBJECT(&s->bar[i]));
> + }
> + object_unparent(OBJECT(&s->rom));
> +
> xen_pt_destroy(d);
> assert(rc);
> - return rc;
> }
>
> static void xen_pt_unregister_device(PCIDevice *d)
> @@ -929,7 +932,7 @@ static void xen_pci_passthrough_class_init(ObjectClass
> *klass, void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> - k->init = xen_pt_initfn;
> + k->realize = xen_pt_realize;
> k->exit = xen_pt_unregister_device;
> k->config_read = xen_pt_pci_read_config;
> k->config_write = xen_pt_pci_write_config;
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index c545280..b4b0c19 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -228,7 +228,7 @@ struct XenPCIPassthroughState {
> bool listener_set;
> };
>
> -int xen_pt_config_init(XenPCIPassthroughState *s);
> +void xen_pt_config_init(XenPCIPassthroughState *s, Error **errp);
> void xen_pt_config_delete(XenPCIPassthroughState *s);
> XenPTRegGroup *xen_pt_find_reg_grp(XenPCIPassthroughState *s, uint32_t
> address);
> XenPTReg *xen_pt_find_reg(XenPTRegGroup *reg_grp, uint32_t address);
> @@ -328,5 +328,6 @@ static inline bool
> is_igd_vga_passthrough(XenHostPCIDevice *dev)
> }
> int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
> int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
> -int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev);
> +int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
> + Error **errp);
> #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index 3d8686d..dad2032 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -1899,8 +1899,9 @@ static uint8_t find_cap_offset(XenPCIPassthroughState
> *s, uint8_t cap)
> return 0;
> }
>
> -static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
> - XenPTRegGroup *reg_grp, XenPTRegInfo *reg)
> +static void xen_pt_config_reg_init(XenPCIPassthroughState *s,
> + XenPTRegGroup *reg_grp, XenPTRegInfo *reg,
> + Error **errp)
> {
> XenPTReg *reg_entry;
> uint32_t data = 0;
> @@ -1919,12 +1920,14 @@ static int
> xen_pt_config_reg_init(XenPCIPassthroughState *s,
> reg_grp->base_offset + reg->offset, &data);
> if (rc < 0) {
> g_free(reg_entry);
> - return rc;
> + error_setg(errp, "Init emulate register fail!");
> + return;
> }
> if (data == XEN_PT_INVALID_REG) {
> /* free unused BAR register entry */
> g_free(reg_entry);
> - return 0;
> + error_setg(errp, "Invalid register value");
> + return;
> }
> /* Sync up the data to dev.config */
> offset = reg_grp->base_offset + reg->offset;
> @@ -1942,7 +1945,8 @@ static int
> xen_pt_config_reg_init(XenPCIPassthroughState *s,
> if (rc) {
> /* Serious issues when we cannot read the host values! */
> g_free(reg_entry);
> - return rc;
> + error_setg(errp, "Cannot read host values!");
> + return;
> }
> /* Set bits in emu_mask are the ones we emulate. The dev.config shall
> * contain the emulated view of the guest - therefore we flip the
> mask
> @@ -1967,10 +1971,10 @@ static int
> xen_pt_config_reg_init(XenPCIPassthroughState *s,
> val = data;
>
> if (val & ~size_mask) {
> - XEN_PT_ERR(&s->dev,"Offset 0x%04x:0x%04x expands past register
> size(%d)!\n",
> - offset, val, reg->size);
> + error_setg(errp, "Offset 0x%04x:0x%04x expands past"
> + " register size(%d)!", offset, val, reg->size);
> g_free(reg_entry);
> - return -ENXIO;
> + return;
> }
> /* This could be just pci_set_long as we don't modify the bits
> * past reg->size, but in case this routine is run in parallel or the
> @@ -1990,11 +1994,9 @@ static int
> xen_pt_config_reg_init(XenPCIPassthroughState *s,
> }
> /* list add register entry */
> QLIST_INSERT_HEAD(®_grp->reg_tbl_list, reg_entry, entries);
> -
> - return 0;
> }
>
> -int xen_pt_config_init(XenPCIPassthroughState *s)
> +void xen_pt_config_init(XenPCIPassthroughState *s, Error **errp)
> {
> int i, rc;
>
> @@ -2039,11 +2041,11 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> reg_grp_offset,
> ®_grp_entry->size);
> if (rc < 0) {
> - XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld, type=0x%x,
> rc:%d\n",
> - i, ARRAY_SIZE(xen_pt_emu_reg_grps),
> + error_setg(errp, "Failed to initialize %d/%ld, type=0x%x,"
> + " rc:%d\n", i, ARRAY_SIZE(xen_pt_emu_reg_grps),
> xen_pt_emu_reg_grps[i].grp_type, rc);
> xen_pt_config_delete(s);
> - return rc;
> + return;
> }
> }
>
> @@ -2054,21 +2056,20 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
> /* initialize capability register */
> for (j = 0; regs->size != 0; j++, regs++) {
> /* initialize capability register */
> - rc = xen_pt_config_reg_init(s, reg_grp_entry, regs);
> - if (rc < 0) {
> - XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld reg
> 0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
> - j,
> ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
> - regs->offset,
> xen_pt_emu_reg_grps[i].grp_type,
> - i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
> + xen_pt_config_reg_init(s, reg_grp_entry, regs, errp);
> + if (*errp) {
> + error_append_hint(errp, "Failed to initialize %d/%ld"
> + " reg 0x%x in grp_type=0x%x (%d/%ld),
> rc=%d\n",
> + j,
> ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
> + regs->offset,
> xen_pt_emu_reg_grps[i].grp_type,
> + i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
> xen_pt_config_delete(s);
> - return rc;
> + return;
> }
> }
> }
> }
> }
> -
> - return 0;
> }
>
> /* delete all emulate register */
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index df6069b..17e7a59 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -161,7 +161,8 @@ struct pci_data {
> uint16_t reserved;
> } __attribute__((packed));
>
> -int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
> +int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
> + Error **errp)
same here about returning void
> {
> unsigned char *bios = NULL;
> struct rom_header *rom;
> @@ -172,12 +173,13 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s,
> XenHostPCIDevice *dev)
> struct pci_data *pd = NULL;
>
> if (!is_igd_vga_passthrough(dev)) {
> + error_setg(errp, "Need to enable igd-passthrough");
> return -1;
> }
>
> bios = get_vgabios(s, &bios_size, dev);
> if (!bios) {
> - XEN_PT_ERR(&s->dev, "VGA: Can't getting VBIOS!\n");
> + error_setg(errp, "VGA: Can't getting VBIOS!");
> return -1;
> }
>
> --
> 2.1.0
>
>
>
