On 01/21/2015 08:00 PM, Ján Tomko wrote:
On 01/21/2015 04:10 AM, lhuang wrote:
On 01/21/2015 03:00 AM, John Ferlan wrote:
On 12/09/2014 01:48 AM, Luyao Huang wrote:
Add a func just check the base target type which
qemu support. But i still doubt this will be useful
, we already have a check when we try to start the
vm. And this check only check the target type, and
the other things will be done in virDomainChrDefParseXML.
The commit message needs some massaging.
Essentially, this patch fixes the "condition" where if a guest has been
started and someone uses attach-device with (or without) the --config
option, then these checks will avoid the "next" guest being modified,
correct?
Right
This will also cause an error earlier that patch 1/2 as
qemuDomainChrInsert is called in the path before qemuDomainAttachDeviceLive
Yes and if this patch have been pushed then the patch 1/2 will be a patch for
improving exist code.
ChrInsert is called after qemuBuildConsoleChrDeviceStr in AttachDevice. We
should error out earlier and include the first patch too.
Thanks for pointing out, error output more earlier more better :)
Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
---
src/qemu/qemu_hotplug.c | 64
+++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b9a0cee..fe91ec7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1384,10 +1384,74 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr
driver,
}
+static int
+qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr)
+{
+ int ret = -1;
+
+ switch (chr->deviceType) {
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+ switch ((virDomainChrSerialTargetType) chr->targetType) {
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
+ ret = 0;
+ break;
+
+ default:
Typically in switches listing other options rather than default:
The point of casting it to virDomainChrSerialTargetType is to catch all the
unhandled values and it only works if there's no default: clause.
Also I don't think we need the ret variable at all. Just return 0 at the end
of the function, or -1 if we reached an unsupported combination.
Good idea ! i will remove the ret and use return in these place.
I think perhaps this one is better than 1/2, but will see if my
responses result in other opinions...
Even if this one fixes the bug, 1/2 is nice to have.
Thanks for pointing out and i forgot cc Jan and Pavel when sent this patch,
maybe they have some other opinions.
No need to cc me, I am subscribed to the list. :)
I just saw other people cc the people who help review the patch when
send a new version
patch to upstream, so i think it is better to cc the people who help
review the patch when write
a new version.
Thanks a lot for your review.
Jan
Luyao
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list