On Wed, Sep 30, 2015 at 09:44:27AM -0400, John Ferlan wrote:
On 09/30/2015 12:43 AM, Martin Kletzander wrote:On Tue, Sep 29, 2015 at 05:27:58PM -0400, John Ferlan wrote:[...]NOTE: The change to the test is because the failure now occurs during parse rather than at run (e.g. earlier, where I think it should).I agree, and this sounds good, I'd have just two minor additions, see below.From a03a8aa073eb410d6b713e6f77572edcf0631263 Mon Sep 17 00:00:00 2001 From: John Ferlan <jferlan@xxxxxxxxxx> Date: Tue, 29 Sep 2015 17:04:11 -0400 Subject: [PATCH] bug 1257844 Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/conf/domain_conf.c | 43 +++++++++++++++++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 393ece7..21439c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3962,6 +3962,41 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } +/* Check if a provided address is valid */ +static bool +virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk)1) The name suggests it checks the address type of any device, I'd somehow add a "Disk" in the name =)yeah - I was merely copying from Ruifeng's code... When I generate an official patch I would attribute the code appropriately...2) Until anything similar to my proposal [1] is added, this would make daemon lose such domain on reload.Hmm.. right... yuck - how easy one forgets about that. Although it does seem you have some traction with the other series. Or perhaps new logic that includes a flag that could get passed along to the PostParse function as well and checked in the "new" else condition from only the Live attach, update, detach paths (seems like overkill though). One thing still not resolved is that outside of virsh edit, one wouldn't be able to remove the device using virsh detach-device
I'd say it's OK that you have to re-define it. You'd have to re-define it anyway. While on that note, I'd gladly welcome any opinions in that thread about the invalid domain XML loading ;)
JohnMartin [1] https://www.redhat.com/archives/libvir-list/2015-September/msg00698.html[...]
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list