Re: [PATCH RESEND] qemu_driver: unlink new domain cfg file when rollback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/31/2016 06:43 AM, Martin Kletzander wrote:
On Fri, Oct 28, 2016 at 04:29:30AM -0700, Michal Privoznik wrote:
On 27.10.2016 17:45, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@xxxxxxxxx>

If we failed to unlink old dom cfg file, we goto rollback.
But inside rollback, we fogot to unlink the new dom cfg file.
This patch fixes this issue.

Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxx>
---
 src/qemu/qemu_driver.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e6f845d..3f4a2fb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19882,6 +19883,11 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
         goto cleanup;
     }

+    if (!(new_dom_cfg_file = virDomainConfigFile(cfg->configDir,
+ new_dom_name))) {
+        goto cleanup;
+    }
+

Whoa, I'm really surprised that our syntax-check does not catch this. It
has a one line body therefore there shouldn't be any curly braces around
it. Also, this could be joined with previous condition.


Actually! =D We have that so "precisely defined" that in this precise
case, it "is optional (not enforced either way)" -- Just to note how
specific we can be =)

It used to be undefined, but according to http://libvirt.org/hacking.html#curly_braces (a change made in in 2014 in commit a9d07d33 by Martin), when the condition spans multiple lines, curly braces are now required:

"Omit the curly braces around an if, while, for etc. body only when both that body *and the condition itself* occupy a single line."

One of the examples following that text is of exactly this situation.

(I've always preferred doing it this way, but have been lazy about it a few times and had it pointed out during review by "The Crusader of the Braces", abologna)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]