[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize()
From: |
Cao jin |
Subject: |
Re: [Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize() |
Date: |
Wed, 23 Dec 2015 21:08:35 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
Hi Stefano,
first of all, thanks for your quick response:)
On 12/23/2015 08:03 PM, Stefano Stabellini wrote:
On Wed, 23 Dec 2015, Cao jin wrote:
Signed-off-by: Cao jin <address@hidden>
---
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.
Ok, I`ll select one style may with the smallest modification;)
Also it might be nice to split the patch in a series.
Yup, and the patches should be independent from each other?
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
really weird...last patch you remind me that it cannot compile, make me
find that my computer didn`t install xen-devel package, then I installed
it right away. But this time, it really can compile on my
computer....anyway, I will check it out later.
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?
I can do that.
[...]
+
if (i != PCI_NUM_REGIONS) {
/* Invalid format or input to short */
- rc = -ENODEV;
+ error_setg(errp, "Invalid format or input to short");
^too short
How about printing all the string in buf? like:
"Invalid format or input to short: %s", buf
for all the other comments below: will fix them up:)
}
out:
close(fd);
- return rc;
}
[...]
--
Yours Sincerely,
Cao Jin