[libvirt] Modern CPU models cannot be used with libvirt
by Jiri Denemark
Hi,
Recently I realized that all modern CPU models defined in
/etc/qemu/target-x86_64.conf are useless when qemu is used through libvirt.
That's because we start qemu with -nodefconfig which results in qemu ignoring
that file with CPU model definitions. We have a very good reason for using
-nodefconfig because we need to control the ABI presented to a guest OS and we
don't want any configuration file that can contain lots of things including
device definitions to be read by qemu. However, we would really like the new
CPU models to be understood by qemu even if used through libvirt. What would
be the best way to solve this?
I suspect this could have been already discussed in the past but obviously a
workable solution was either not found or just not implemented.
Jirka
12 years, 7 months
[libvirt] RFC: storage migration via pre-copy and streaming mirrors
by Eric Blake
This is the counter-proposal to my earlier RFC for storage migration via
snapshot mirrors[1], resulting from a NACK on the code review for that
earlier proposal[2]. In particular, this proposal fleshes out some of
Paolo's design overview on the qemu wiki[3].
[1] https://www.redhat.com/archives/libvir-list/2012-March/msg00578.html
[2] https://www.redhat.com/archives/libvir-list/2012-March/msg01033.html
[3] http://wiki.qemu.org/Features/SnapshotsMultipleDevices
My plan is to have everything in this RFC coded up in the next couple of
days (hopefully no later than Thursday); this has missed the feature
freeze for 0.9.11, so it should not be applied upstream until after the
weekend release, as one of the first patches for 0.9.12. Backport-wise,
the new flags can be backported as far back as the 0.9.10 .so API, but
the new virDomainBlockCopy() API cannot be exported when doing a
backport without breaking .so versions (although it's implemenation can
be used internally).
Additions
=========
The following new error code will be added:
VIR_ERR_BLOCK_COPY_IN_PROGRESS
The following new API will be added:
int virDomainBlockCopy(virDomainPtr dom,
const char *disk,
const char *base,
const char *dest,
const char *format,
unsigned long bandwidth,
unsigned int flags);
The following new named values will be added:
enum virDomainBlockJobType (used in virDomainBlockJobInfo):
VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2
The following new flags will be added:
for virDomainBlockRebase:
VIR_DOMAIN_BLOCK_REBASE_SHALLOW = 1 << 0
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT = 1 << 1
VIR_DOMAIN_BLOCK_REBASE_COPY = 1 << 2
for virDomainBlockCopy:
VIR_DOMAIN_BLOCK_COPY_SHALLOW = 1 << 0
VIR_DOMAIN_BLOCK_COPY_REUSE_EXT = 1 << 1
for virDomainBlockJobAbort:
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
Add some XML:
Under //domain/drivers/disk, next to <source file='...'/>, add <mirror
file='...'/>
Semantics
=========
virDomainBlockCopy sets up a BLOCK_JOB_TYPE_COPY job. 'disk' names the
disk to be copied (can be 'vda' or '/path/to/source', as with other
block commands) and must not be NULL. 'base' names the path to the
backing file in the chain of the source that will be the new backing
file of the destination; if this parameter is NULL, then the destination
file defaults to a complete block pull, but the COPY_SHALLOW flag
instead requests a pull of just the top file in the source backing
chain. 'dest' names the copy being created and must not be NULL;
normally, this file is created by the hypervisor/libvirt, but the
COPY_REUSE_EXT flag lets an application pass in a pre-created file
(allowing metadata to include a relative instead of absolute backing
file name). 'format' gives the format of the copy, or NULL to either
probe the format of a COPY_REUSE_EXT dest or to reuse the same format as
the source. flags cannot contain COPY_SHALLOW unless 'base' is NULL.
Once a block copy job is started, calls to virDomainGetBlockJobInfo()
for the same 'disk' will report an info with
VIR_DOMAIN_BLOCK_JOB_TYPE_COPY as the type. This job never completes on
its own, but must be stopped by the user (this enables mirroring to
continue until the user informs libvirt that any backing files, perhaps
located at different locations as specified by relative path names using
REUSE_EXT, have been externally copied into place). There are two
phases to a TYPE_COPY job. In the first phase, cur < end when querying
progress, calls to virDomainBlockJobAbort(dom, disk, 0) will cancel the
operation and revert to the source, and calls to
virDomainBlockJobAbort(dom, disk, VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) will
fail with VIR_ERR_BLOCK_COPY_IN_PROGRESS. In the second phase, cur ==
end when querying progress, calls to virDomainBlockJobAbort(dom, disk,
0) will break the mirroring and revert to the source, while calls to
virDomainBlockJobAbort(dom, disk, VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) will
break the mirroring and pivot to the destination. Use of
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT on a non-copy block job will fail with
VIR_ERR_INVALID_ARG.
virDomainBlockRebase(dom, disk, dest, bandwidth,
VIR_DOMAIN_BLOCK_REBASE_COPY | (flags & 3)) is shorthand for
virDomainBlockCopy(dom, disk, NULL, dest, NULL, bandwidth, (flags & 3))
- that is, use of the REBASE_COPY flag treats the BlockRebase 'base'
argument as the BlockCopy 'dest' argument, creates the destination with
the same file format as the source (or probes the backing file format if
REBASE_REUSE_EXT is used), and passes the COPY_SHALLOW and
COPY_REUSE_EXT flags through (note that the similarly named flags were
conveniently chosen to be the same values). Attempts to use
REBASE_SHALLOW or REBASE_REUSE_EXT without also using REBASE_COPY will
fail with VIR_ERR_INVALID_ARG.
While a copy operation is in place, virDomainGetXMLDesc (dumpxml) will
show the <mirror> element for that <disk>.
Initial Implementation
======================
When virDomainBlockCopy is called (perhaps via the virDomainBlockRebase
alias), it first sets up a mirror using the 'drive-mirror' monitor
command and the destination file name. The mirror is opened with the
'existing' mode if _REUSE_EXT is present; otherwise it is opened with
the 'absolute-paths' mode if _SHALLOW is present or 'no-backing-file'
mode if no flags are present. Next, the function calls the
'block_stream' monitor command to start the streaming. The streaming
command uses 'base' as its starting point, except that when _SHALLOW was
specified, libvirt will use the backing file of the source disk, rather
than NULL (this can be obtained by the 'query-block' monitor command;
although someday libvirt should start tracking this information in
<domain> XML rather than relying on qemu). At this point, control
returns to the user, and the stream proceeds in the background;
virDomainBlockJobSetSpeed can tune the speed of the block streaming.
At least in the initial implementation, as long as the block job is
active, libvirt will prevent 'virDomainMigrate', 'virDomainSave',
'virDomainSnapshotCreateXML' in general, and 'virDomainDetachDevice' of
the disk being mirrored, all with the new VIR_ERR_BLOCK_COPY_IN_PROGRESS
error. This is because I don't have an easy way to resume mirroring
when restarting a new qemu process; preventing these actions until the
user first cancels the ongoing mirroring will result in fewer corner
cases that libvirt has to worry about. This also implies that the
initial implementation will fail for persistent domains, and only be
useful for transient domains. Attempts to define a domain with a
<mirror> element are rejected, leaving <mirror> as output-only XML
useful in restoring state when restarting libvirtd. It may be possible
to add persistent support in the future, once we determine how to make
qemu resume a mirrored block device; at that point, it would be possible
to specify <mirror> in domain xml during domain creation or during
device hotplug.
When the block stream finishes, qemu will send an event to libvirt
(libvirt will also have to manually check for completion on a libvirtd
restart, based on whether cur == end in the block job info). I'm not
yet sure whether to expose this event to the user so that they do not
have to poll the block job info, or whether to consume it internally.
At any rate, before this event occurs, the BLOCK_JOB_ABORT_PIVOT flag is
rejected, and virDomainBlockJobAbort without flags uses the
'block_job_cancel' monitor command to stop the streaming early, then the
'drive-reopen' monitor command to break the mirroring back to the
source; it is feasible that there is a race where a 'block_job_cancel'
can be called after the pull is complete but before the completion event
has been processed, so the code must proceed on to the 'drive-reopen'
even if the job cancel fails. After the event occurs,
virDomainBlockJobAbort only needs to use the 'drive-reopen' monitor
command, with either the source or the destination file depending on the
BLOCK_JOB_ABORT_PIVOT flag.
Until 'drive-reopen' is made atomic in qemu (by adding code to support
it inside 'transaction'), the user risks a block job abort rendering the
disk unusable, because the source was closed before the destination was
opened; hopefully this situation is rare, in part because libvirt will
do stat() checks and SELinux labeling on destination files before
starting qemu monitor commands, as a sanity check that qemu will be able
to use the specified files. If qemu ever adds atomic 'drive-reopen'
support, we can add a new flag BLOCK_JOB_ABORT_ATOMIC that fails on
older qemu, and ensures the use of 'transaction' on the newer qemu that
supports an atomic reopen.
If a block mirror is aborted (whether by the user calling
virDomainBlockJobAbort with no flag, or by the qemu process ending due
to things like a guest-initiated shutdown), then the mirror can be
safely discarded, and restarting the domain will be unmirrored where the
virDomainBlockRebase can be called again from scratch.
Examples
========
For some examples, starting with base <- snap1 <- snap2 <- snap3 as the
backing chain for disk 'vda',
virDomainBlockCopy(dom, "vda", NULL, "/path/to/copy", NULL, 0)
would set up a job that results in /path/to/copy being the same file
format as snap3, but containing the entire chain
virDomainBlockCopy(dom, "vda", "/path/to/snap1", "/path/to/copy",
"qed", 0)
would set up a job that results in base <- snap1 <- copy as the mirrored
backing chain, and ensuring that copy is formatted as qed regardless of
the format of snap3
virDomainBlockCopy(dom, "vda", NULL, "/path/to/copy",
NULL, VIR_DOMAIN_BLOCK_COPY_SHALLOW)
is shorthand for
virDomainBlockCopy(dom, "vda", "/path/to/snap2", "/path/to/copy",
NULL, 0)
and results in base <- snap1 <- snap2 <- copy creates copy using the
same format as snap3
virDomainBlockCopy(dom, "vda", "/path/to/snap2", "/path/to/copy",
NULL, VIR_DOMAIN_BLOCK_COPY_REUSE_EXT)
requires /path/to/copy to already exist, probes it for existing format
(which might be different from snap3), and proceeds to mirror everything
so that snap2 is the base of copy (and the user is at fault if the
pre-existing file doesn't call out a backing file that happens to be
identical in content to snap2)
oVirt will probably use the sequence:
- use qemu-img to create an empty qcow2 file with relative backing name
to the destination storage
- call virDomainBlockRebase(dom, disk, "/path/to/copy",
VIR_DOMAIN_BLOCK_REBASE_COPY | VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)
- copy the base files from source to destination storage (this can be
done in parallel, either before or after the virDomainBlockRebase call)
- wait for the block pull to finish (either by waiting for an event if I
propagate the event, or by polling virDomainBlockJobInfo, or even by
polling virDomainBlockJobAbort(VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) and
checking for VIR_ERR_BLOCK_COPY_IN_PROGRESS
- once both the base files are in place and the block pull half of the
copy job is complete (and without regard to whether the block stream or
the external base file copying completed first), call
virDomainBlockJobAbort(,VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) to reopen to
the new storage domain chain
Comparison to first RFC
=======================
This proposal exposes only one disk at a time, while the earlier
virDomainSnapshotCreateXML <mirror> approach could atomically set up
mirroring on multiple disks. However, the nature of block jobs being a
background process means that parallel jobs can be run on independent
disks, so the user can do the overall block migration with the time cost
of the slowest disk, rather than having to do things serially with the
time cost of all disks added together.
This proposal avoids having to create an intermediate snapshot, so the
pull is more efficient and the source chain does not get longer, no
matter how many times the process is aborted and restarted.
This proposal can expose the no-backing-file mode, while the snapshot
approach did not.
To survive across libvirtd restarts, the snapshot approach was using
<domainsnapshot> to store the mirroring status in a user-visible
location; this approach has to modify the internal live xml (alongside
other internal data, such as the qemu pid). Or perhaps I can add a
<mirror> subelement to <disk> of a domain and make it user-visible after
all, and treat that as an output-only parameter for now.
Both approaches face the dilemma of how to start a new qemu process with
mirroring intact, and my solution in both patch series will be to
prevent any action that would force libvirt to save domain state until
after the user has first canceled all current mirroring jobs. This
limitation is not permanent - if future qemu provides better ways to
restart mirroring, and as libvirt is taught to store the full backing
chain in <domain> xml instead of probing it on the fly, we can relax
this restriction in the future.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12 years, 7 months
[libvirt] libvirt starts QEMU with bad CPU based on its XML
by Doug Goldstein
My XML configs for my VM machines have it set to AMD Opteron G3s, but
libvirt is creating QEMU with a core2duo. This is not correct at all.
I've included the setup and config of one machine that does this. But
another machine that does this matches danpb's AMD Quad Core Dual NUMA
system he posted on the mailing list recently. That machine is using a
stock qemu-0.15.1 from upstream's git and libvirt git a few commits
shy of the 0.9.11 rc1. The machine below is using libvirt 0.9.10 and
qemu-kvm-1.0.
Here's the setup:
The host is:
processor : 63
vendor_id : AuthenticAMD
cpu family : 21
model : 1
model name : AMD Opteron(TM) Processor 6272
stepping : 2
microcode : 0x6000613
cpu MHz : 2099.875
cache size : 2048 KB
physical id : 2
siblings : 16
core id : 7
cpu cores : 8
apicid : 79
initial apicid : 79
fpu : yes
fpu_exception : yes
cpuid level : 13
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt
pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc extd_apicid
amd_dcm aperfmperf pni pclmulqdq monitor ssse3 cx16 sse4_1 sse4_2
popcnt aes xsave avx lahf_lm cmp_legacy svm extapic cr8_legacy abm
sse4a misalignsse 3dnowprefetch osvw ibs xop skinit wdt lwp fma4
nodeid_msr topoext perfctr_core arat cpb npt lbrv svm_lock nrip_save
tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold
bogomips : 4200.08
TLB size : 1536 4K pages
clflush size : 64
cache_alignment : 64
address sizes : 48 bits physical, 48 bits virtual
power management: ts ttp tm 100mhzsteps hwpstate [9]
The guest is:
processor : 1
vendor_id : AuthenticAMD
cpu family : 6
model : 15
model name : Intel(R) Core(TM)2 Duo CPU T7700 @ 2.40GHz
stepping : 11
cpu MHz : 2100.397
cache size : 512 KB
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 10
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov pat
pse36 clflush mmx fxsr sse sse2 syscall nx mmxext fxsr_opt pdpe1gb lm
pni cx16 popcnt lahf_lm cmp_legacy svm cr8legacy abm sse4a misalignsse
3dnowprefetch
bogomips : 4200.01
The QEMU command line contains the following:
-cpu core2duo,+wdt,+skinit,+osvw,+3dnowprefetch,+misalignsse,+sse4a,+abm,+cr8legacy,+extapic,+svm,+cmp_legacy,+lahf_lm,+rdtscp,+pdpe1gb,+fxsr_opt,+mmxext,+aes,+popcnt,+sse4.2,+sse4.1,+cx16,+ht
While libvirt's XML contains:
<cpu match='exact'>
<model>Opteron_G3</model>
<vendor>AMD</vendor>
<feature policy='require' name='aes'/>
<feature policy='require' name='skinit'/>
<feature policy='require' name='vme'/>
<feature policy='require' name='mmxext'/>
<feature policy='require' name='fxsr_opt'/>
<feature policy='require' name='cr8legacy'/>
<feature policy='require' name='ht'/>
<feature policy='require' name='3dnowprefetch'/>
<feature policy='require' name='ssse3'/>
<feature policy='require' name='wdt'/>
<feature policy='require' name='extapic'/>
<feature policy='require' name='pdpe1gb'/>
<feature policy='require' name='osvw'/>
<feature policy='require' name='sse4.1'/>
<feature policy='require' name='cmp_legacy'/>
<feature policy='require' name='sse4.2'/>
</cpu>
--
Doug Goldstein
12 years, 7 months
[libvirt] [PATCH] virsh: plug memory leaks on failure path
by Alex Jia
Leaks are introduced in commit 1cf0e3d and fe383bb.
* tools/virsh.c (cmdSnapshotList): fix memory leaks.
* How to reproduce?
% virsh snapshot-list foo --parent --roots
error: --parent and --roots are mutually exclusive
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)
% virsh snapshot-list foo --parent --tree
error: --parent and --tree are mutually exclusive
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)
% virsh snapshot-list foo --roots --tree
error: --roots and --tree are mutually exclusive
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)
......
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/virsh.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 5009b6b..f5c3b60 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -16357,11 +16357,13 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "roots")) {
vshError(ctl, "%s",
_("--parent and --roots are mutually exclusive"));
+ virDomainFree(dom);
return false;
}
if (tree) {
vshError(ctl, "%s",
_("--parent and --tree are mutually exclusive"));
+ virDomainFree(dom);
return false;
}
parent_filter = 1;
@@ -16369,11 +16371,13 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (tree) {
vshError(ctl, "%s",
_("--roots and --tree are mutually exclusive"));
+ virDomainFree(dom);
return false;
}
if (from) {
vshError(ctl, "%s",
_("--roots and --from are mutually exclusive"));
+ virDomainFree(dom);
}
flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
}
@@ -16381,6 +16385,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (tree) {
vshError(ctl, "%s",
_("--leaves and --tree are mutually exclusive"));
+ virDomainFree(dom);
return false;
}
flags |= VIR_DOMAIN_SNAPSHOT_LIST_LEAVES;
--
1.7.1
12 years, 7 months
[libvirt] [PATCH] virsh: add error return value
by Alex Jia
* tools/virsh.c (cmdSnapshotList): Avoiding to continue to execute subsequent
codes, the programming should return false when meeting a error.
For details, please see the following link:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=807545
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/virsh.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 5009b6b..7e74744 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -16374,6 +16374,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (from) {
vshError(ctl, "%s",
_("--roots and --from are mutually exclusive"));
+ return false;
}
flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
}
--
1.7.1
12 years, 7 months
[libvirt] Coverity automatic detection
by Alex Jia
This email is automatically generated.
The test result is based on the following git commit:
851117b Enable all warnings permanently & default to -Werror for GIT builds
Analysis summary report:
------------------------
Files analyzed : 306
Total LoC input to cov-analyze : 362576
Functions analyzed : 8807
Paths analyzed : 1075505
Defect occurrences found : 105 Total
7 ATOMICITY
15 CHECKED_RETURN
14 DEADCODE
1 EVALUATION_ORDER
9 FORWARD_NULL
13 LOCK
3 NEGATIVE_RETURNS
1 NO_EFFECT
3 NULL_RETURNS
1 OVERRUN_STATIC
16 RESOURCE_LEAK
1 RETURN_LOCAL
12 REVERSE_INULL
1 SIZEOF_MISMATCH
6 UNINIT
2 UNUSED_VALUE
Exceeded path limit of 5000 paths in 0.55% of functions (normally up to 5% of functions encounter this limitation)
For details, please see attachment.
Regards,
Alex
12 years, 7 months
[libvirt] [PATCH] snapshot: fix virsh docs
by Eric Blake
Commit d42a2ff forgot to touch up virsh documentation, and commit
4e9953a mis-spelled the option name.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Fix typo
and match recent change in flag meaning.
---
tools/virsh.pod | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 390bcec..76d2ea4 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2347,7 +2347,7 @@ metadata again).
If I<--reuse-external> is specified, and the snapshot XML requests an
external snapshot with a destination of an existing file, then the
-existing file is truncated and reused; otherwise, a snapshot is refused
+destination must exist, and is reused; otherwise, a snapshot is refused
to avoid losing contents of the existing files.
If I<--quiesce> is specified, libvirt will try to use guest agent
@@ -2367,7 +2367,7 @@ metadata is silently lost when the domain quits running (whether
by command such as B<destroy> or by internal guest action).
=item B<snapshot-create-as> I<domain> {[I<--print-xml>]
-| [I<--no-metadata>] [I<--halt>] [I<--reuse-existing>]} [I<name>]
+| [I<--no-metadata>] [I<--halt>] [I<--reuse-external>]} [I<name>]
[I<description>] [I<--disk-only> [I<--quiesce>] [I<--atomic>]
[[I<--diskspec>] B<diskspec>]...]
@@ -2394,7 +2394,7 @@ results in the following XML:
If I<--reuse-external> is specified, and the domain XML or I<diskspec>
option requests an external snapshot with a destination of an existing
-file, then the existing file is truncated and reused; otherwise, a
+file, then the destination must exist, and is reused; otherwise, a
snapshot is refused to avoid losing contents of the existing files.
If I<--quiesce> is specified, libvirt will try to use guest agent
--
1.7.7.6
12 years, 7 months
[libvirt] Turning off libvirtd mdns by default
by Stef Walter
In the GNOME UI we'd like to make use of Avahi discovery and name
resolution "out of the box". A typical use case is for discovery of
printers that are advertised using MDNS. This should work even on
potentially 'hostile' networks such as a wireless access point in a
print shop or airport. It should work without user configuration.
https://fedoraproject.org/wiki/Desktop/Whiteboards/AvahiDefault
In order to turn on Avahi by default, and make it work by default, we'd
like to make it possible to use Avahi without advertising any
information to the network by default. Advertising information to the
network (even the host name) without the user's configuration or consent
is a privacy issue.
libvirtd advertises itself via MDNS on the network by default. I
understand that MDNS discovery of libvirtd is really handy in many cases.
However since one has to configure network access in libvirtd anyway --
none of the access methods work "out of the box" to my understanding --
I'd like to suggest turning off libvirtd's MDNS publishing by default.
As part of setting up libvirtd for network access, the user would turn
on mdns_adv.
I hope that makes sense. Let me know if I've gotten something wrong.
Would you accept a patch to do this? Or would you suggest that we try
and do this downstream in the Fedora/RHEL packages instead?
Cheers,
Stef
12 years, 7 months
[libvirt] [PATCH] qemu: Make migration fail when port profile association fails on the dst host
by Christian Benvenuti (benve)
In the current V3 migration protocol, Libvirt does not
check the result of the function
qemuMigrationVPAssociatePortProfiles
This means that it is possible for a migration to complete
successfully even when the VM loses network connectivity on
the destination host.
With this change libvirt aborts the migration
(during the "finish" step) when the above function fails, that
is to say when at least one of the port profile associations fails.
Signed-off by: Christian Benvenuti <benve(a)cisco.com>
---
src/qemu/qemu_migration.c | 24 ++++++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 802785f..38fbf83 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2716,7 +2716,7 @@ qemuMigrationPerform(struct qemud_driver *driver,
}
}
-static void
+static int
qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {
int i;
int last_good_net = -1;
@@ -2731,13 +2731,17 @@
qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {
virDomainNetGetActualDirectDev(net),
-1,
def->uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) < 0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH,
+ false) < 0) {
+ VIR_ERROR("Port profile Associate failed for %s",
net->ifname);
goto err_exit;
+ }
+ VIR_DEBUG("Port profile Associate succeeded for %s",
net->ifname);
}
last_good_net = i;
}
- return;
+ return 0;
err_exit:
for (i = 0; i < last_good_net; i++) {
@@ -2751,6 +2755,7 @@ err_exit:
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
}
}
+ return -1;
}
@@ -2805,7 +2810,18 @@ qemuMigrationFinish(struct qemud_driver *driver,
goto endjob;
}
- qemuMigrationVPAssociatePortProfiles(vm->def);
+ if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0) {
+ qemuProcessStop(driver, vm, 1, VIR_DOMAIN_SHUTOFF_FAILED);
+ virDomainAuditStop(vm, "failed");
+ event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+
VIR_DOMAIN_EVENT_STOPPED_FAILED);
+ goto endjob;
+ }
if (flags & VIR_MIGRATE_PERSIST_DEST) {
virDomainDefPtr vmdef;
--
1.7.1
12 years, 7 months
[libvirt] [PATCH] Enable all warnings permanently & default to -Werror for GIT builds
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Given that we auto-detect whether each -Wxxxx flag is supported by
GCC, and we are warning-free and use automake silent rules, there
is no compelling reason to allow compile warnings to be disabled.
Replace the --enable-compile-warnings flag with a simpler
--enable-werror flag, which defaults to 'yes' if building
from GIT, or 'no' if building from tar.gz
This helps ensure that everyone writing patches for libvirt will
take care to fix their warning problems before submitting for
review
* autobuild.sh: Force -Werror
* configure.ac: Update for LIBVIRT_COMPILE_WARNINGS macro change
* m4/virt-compile-warnings.m4: Permanently enable all warnings,
auto-enable Werror for GIT builds
---
autobuild.sh | 4 +-
configure.ac | 4 +-
m4/virt-compile-warnings.m4 | 258 +++++++++++++++++++++----------------------
3 files changed, 130 insertions(+), 136 deletions(-)
diff --git a/autobuild.sh b/autobuild.sh
index 1c289b4..7e8f645 100755
--- a/autobuild.sh
+++ b/autobuild.sh
@@ -20,7 +20,7 @@ cd build
../autogen.sh --prefix="$AUTOBUILD_INSTALL_ROOT" \
--enable-test-coverage \
--disable-nls \
- --enable-compile-warnings=error
+ --enable-werror
# If the MAKEFLAGS envvar does not yet include a -j option,
# add -jN where N depends on the number of processors.
@@ -74,7 +74,7 @@ if [ -x /usr/bin/i686-pc-mingw32-gcc ]; then
--build=$(uname -m)-pc-linux \
--host=i686-pc-mingw32 \
--prefix="$AUTOBUILD_INSTALL_ROOT/i686-pc-mingw32/sys-root/mingw" \
- --enable-compile-warnings=error \
+ --enable-werror \
--without-libvirtd \
--without-python
diff --git a/configure.ac b/configure.ac
index 740129c..32cc8d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -106,7 +106,7 @@ VERSION_SCRIPT_FLAGS=-Wl,--version-script=
VERSION_SCRIPT_FLAGS="-Wl,-M -Wl,"
AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS])
-LIBVIRT_COMPILE_WARNINGS([maximum])
+LIBVIRT_COMPILE_WARNINGS
AC_MSG_CHECKING([for CPUID instruction])
AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
@@ -2820,7 +2820,7 @@ AC_MSG_NOTICE([])
AC_MSG_NOTICE([Miscellaneous])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([ Debug: $enable_debug])
-AC_MSG_NOTICE([ Warnings: $enable_compile_warnings])
+AC_MSG_NOTICE([ Use -Werror: $set_werror])
AC_MSG_NOTICE([Warning Flags: $WARN_CFLAGS])
AC_MSG_NOTICE([ Readline: $lv_use_readline])
AC_MSG_NOTICE([ Python: $with_python])
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 3a428c3..5527bff 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -7,139 +7,133 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
dnl More compiler warnings
dnl ******************************
- AC_ARG_ENABLE(compile-warnings,
- [AC_HELP_STRING([--enable-compile-warnings=@<:@no/yes/error@:>@],
- [Turn on compiler warnings])],,
- [enable_compile_warnings="m4_default([$1],[yes])"])
-
- case "$enable_compile_warnings" in
- no)
- try_compiler_flags=""
- ;;
- yes|minimum|maximum|error)
-
- # List of warnings that are not relevant / wanted
-
- # Don't care about C++ compiler compat
- dontwarn="$dontwarn -Wc++-compat"
- dontwarn="$dontwarn -Wabi"
- dontwarn="$dontwarn -Wdeprecated"
- # Don't care about ancient C standard compat
- dontwarn="$dontwarn -Wtraditional"
- # Don't care about ancient C standard compat
- dontwarn="$dontwarn -Wtraditional-conversion"
- # Ignore warnings in /usr/include
- dontwarn="$dontwarn -Wsystem-headers"
- # Happy for compiler to add struct padding
- dontwarn="$dontwarn -Wpadded"
- # GCC very confused with -O2
- dontwarn="$dontwarn -Wunreachable-code"
- # Too many to deal with
- dontwarn="$dontwarn -Wconversion"
- # Too many to deal with
- dontwarn="$dontwarn -Wsign-conversion"
- # GNULIB gettext.h violates
- dontwarn="$dontwarn -Wvla"
- # Many GNULIB header violations
- dontwarn="$dontwarn -Wundef"
- # Need to allow bad cast for execve()
- dontwarn="$dontwarn -Wcast-qual"
- # We need to use long long in many places
- dontwarn="$dontwarn -Wlong-long"
- # We allow manual list of all enum cases without default:
- dontwarn="$dontwarn -Wswitch-default"
- # We allow optional default: instead of listing all enum values
- dontwarn="$dontwarn -Wswitch-enum"
- # Not a problem since we don't use -fstrict-overflow
- dontwarn="$dontwarn -Wstrict-overflow"
- # Not a problem since we don't use -funsafe-loop-optimizations
- dontwarn="$dontwarn -Wunsafe-loop-optimizations"
- # Things like virAsprintf mean we can't use this
- dontwarn="$dontwarn -Wformat-nonliteral"
-
- # We might fundamentally need some of these disabled forever, but
- # ideally we'd turn many of them on
- dontwarn="$dontwarn -Wfloat-equal"
- dontwarn="$dontwarn -Wdeclaration-after-statement"
- dontwarn="$dontwarn -Wcast-qual"
- dontwarn="$dontwarn -Wconversion"
- dontwarn="$dontwarn -Wsign-conversion"
- dontwarn="$dontwarn -Wpacked"
- dontwarn="$dontwarn -Wunused-macros"
- dontwarn="$dontwarn -Woverlength-strings"
- dontwarn="$dontwarn -Wstack-protector"
-
- # Get all possible GCC warnings
- gl_MANYWARN_ALL_GCC([maybewarn])
-
- # Remove the ones we don't want, blacklisted earlier
- gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn])
-
- # Check for $CC support of each warning
- for w in $wantwarn; do
- gl_WARN_ADD([$w])
- done
-
- # GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff.
- # Unfortunately, this means you can't simply use '-Wsign-compare'
- # with gl_MANYWARN_COMPLEMENT
- # So we have -W enabled, and then have to explicitly turn off...
- gl_WARN_ADD([-Wno-sign-compare])
-
- # GNULIB expects this to be part of -Wc++-compat, but we turn
- # that one off, so we need to manually enable this again
- gl_WARN_ADD([-Wjump-misses-init])
-
- # GNULIB turns on -Wformat=2 which implies -Wformat-nonliteral,
- # so we need to manually re-exclude it.
- gl_WARN_ADD([-Wno-format-nonliteral])
-
- # This should be < 256 really. Currently we're down to 4096,
- # but using 1024 bytes sized buffers (mostly for virStrerror)
- # stops us from going down further
- gl_WARN_ADD([-Wframe-larger-than=4096])
- dnl gl_WARN_ADD([-Wframe-larger-than=256])
-
- # Silence certain warnings in gnulib, and use improved glibc headers
- AC_DEFINE([lint], [1],
- [Define to 1 if the compiler is checking for lint.])
- AC_DEFINE([_FORTIFY_SOURCE], [2],
- [enable compile-time and run-time bounds-checking, and some warnings])
-
- # Extra special flags
- dnl -fstack-protector stuff passes gl_WARN_ADD with gcc
- dnl on Mingw32, but fails when actually used
- case $host in
- *-*-linux*)
- dnl Fedora only uses -fstack-protector, but doesn't seem to
- dnl be great overhead in adding -fstack-protector-all instead
- dnl gl_WARN_ADD([-fstack-protector])
- gl_WARN_ADD([-fstack-protector-all])
- gl_WARN_ADD([--param=ssp-buffer-size=4])
- ;;
- esac
- gl_WARN_ADD([-fexceptions])
- gl_WARN_ADD([-fasynchronous-unwind-tables])
- gl_WARN_ADD([-fdiagnostics-show-option])
- gl_WARN_ADD([-funit-at-a-time])
-
- # Need -fipa-pure-const in order to make -Wsuggest-attribute=pure
- # fire even without -O.
- gl_WARN_ADD([-fipa-pure-const])
- # We should eventually enable this, but right now there are at
- # least 75 functions triggering warnings.
- gl_WARN_ADD([-Wno-suggest-attribute=pure])
- gl_WARN_ADD([-Wno-suggest-attribute=const])
-
- if test "$enable_compile_warnings" = "error"
- then
- gl_WARN_ADD([-Werror])
- fi
- ;;
- *)
- AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to --enable-compile-warnings)
- ;;
+ AC_ARG_ENABLE([werror],
+ AS_HELP_STRING([--enable-werror], [Use -Werror (if supported)]),
+ [set_werror="$enableval"],
+ [if test -d $srcdir/.git; then
+ is_git_version=true
+ set_werror=yes
+ else
+ set_werror=no
+ fi])
+
+ # List of warnings that are not relevant / wanted
+
+ # Don't care about C++ compiler compat
+ dontwarn="$dontwarn -Wc++-compat"
+ dontwarn="$dontwarn -Wabi"
+ dontwarn="$dontwarn -Wdeprecated"
+ # Don't care about ancient C standard compat
+ dontwarn="$dontwarn -Wtraditional"
+ # Don't care about ancient C standard compat
+ dontwarn="$dontwarn -Wtraditional-conversion"
+ # Ignore warnings in /usr/include
+ dontwarn="$dontwarn -Wsystem-headers"
+ # Happy for compiler to add struct padding
+ dontwarn="$dontwarn -Wpadded"
+ # GCC very confused with -O2
+ dontwarn="$dontwarn -Wunreachable-code"
+ # Too many to deal with
+ dontwarn="$dontwarn -Wconversion"
+ # Too many to deal with
+ dontwarn="$dontwarn -Wsign-conversion"
+ # GNULIB gettext.h violates
+ dontwarn="$dontwarn -Wvla"
+ # Many GNULIB header violations
+ dontwarn="$dontwarn -Wundef"
+ # Need to allow bad cast for execve()
+ dontwarn="$dontwarn -Wcast-qual"
+ # We need to use long long in many places
+ dontwarn="$dontwarn -Wlong-long"
+ # We allow manual list of all enum cases without default:
+ dontwarn="$dontwarn -Wswitch-default"
+ # We allow optional default: instead of listing all enum values
+ dontwarn="$dontwarn -Wswitch-enum"
+ # Not a problem since we don't use -fstrict-overflow
+ dontwarn="$dontwarn -Wstrict-overflow"
+ # Not a problem since we don't use -funsafe-loop-optimizations
+ dontwarn="$dontwarn -Wunsafe-loop-optimizations"
+ # Things like virAsprintf mean we can't use this
+ dontwarn="$dontwarn -Wformat-nonliteral"
+
+ # We might fundamentally need some of these disabled forever, but
+ # ideally we'd turn many of them on
+ dontwarn="$dontwarn -Wfloat-equal"
+ dontwarn="$dontwarn -Wdeclaration-after-statement"
+ dontwarn="$dontwarn -Wcast-qual"
+ dontwarn="$dontwarn -Wconversion"
+ dontwarn="$dontwarn -Wsign-conversion"
+ dontwarn="$dontwarn -Wpacked"
+ dontwarn="$dontwarn -Wunused-macros"
+ dontwarn="$dontwarn -Woverlength-strings"
+ dontwarn="$dontwarn -Wstack-protector"
+
+ # Get all possible GCC warnings
+ gl_MANYWARN_ALL_GCC([maybewarn])
+
+ # Remove the ones we don't want, blacklisted earlier
+ gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn])
+
+ # Check for $CC support of each warning
+ for w in $wantwarn; do
+ gl_WARN_ADD([$w])
+ done
+
+ # GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff.
+ # Unfortunately, this means you can't simply use '-Wsign-compare'
+ # with gl_MANYWARN_COMPLEMENT
+ # So we have -W enabled, and then have to explicitly turn off...
+ gl_WARN_ADD([-Wno-sign-compare])
+
+ # GNULIB expects this to be part of -Wc++-compat, but we turn
+ # that one off, so we need to manually enable this again
+ gl_WARN_ADD([-Wjump-misses-init])
+
+ # GNULIB turns on -Wformat=2 which implies -Wformat-nonliteral,
+ # so we need to manually re-exclude it.
+ gl_WARN_ADD([-Wno-format-nonliteral])
+
+ # This should be < 256 really. Currently we're down to 4096,
+ # but using 1024 bytes sized buffers (mostly for virStrerror)
+ # stops us from going down further
+ gl_WARN_ADD([-Wframe-larger-than=4096])
+ dnl gl_WARN_ADD([-Wframe-larger-than=256])
+
+ # Silence certain warnings in gnulib, and use improved glibc headers
+ AC_DEFINE([lint], [1],
+ [Define to 1 if the compiler is checking for lint.])
+ AC_DEFINE([_FORTIFY_SOURCE], [2],
+ [enable compile-time and run-time bounds-checking, and some warnings])
+
+ # Extra special flags
+ dnl -fstack-protector stuff passes gl_WARN_ADD with gcc
+ dnl on Mingw32, but fails when actually used
+ case $host in
+ *-*-linux*)
+ dnl Fedora only uses -fstack-protector, but doesn't seem to
+ dnl be great overhead in adding -fstack-protector-all instead
+ dnl gl_WARN_ADD([-fstack-protector])
+ gl_WARN_ADD([-fstack-protector-all])
+ gl_WARN_ADD([--param=ssp-buffer-size=4])
+ ;;
esac
+ gl_WARN_ADD([-fexceptions])
+ gl_WARN_ADD([-fasynchronous-unwind-tables])
+ gl_WARN_ADD([-fdiagnostics-show-option])
+ gl_WARN_ADD([-funit-at-a-time])
+
+ # Need -fipa-pure-const in order to make -Wsuggest-attribute=pure
+ # fire even without -O.
+ gl_WARN_ADD([-fipa-pure-const])
+ # We should eventually enable this, but right now there are at
+ # least 75 functions triggering warnings.
+ gl_WARN_ADD([-Wno-suggest-attribute=pure])
+ gl_WARN_ADD([-Wno-suggest-attribute=const])
+
+ if test "$set_werror" = "yes"
+ then
+ gl_WARN_ADD([-Werror])
+ fi
WARN_LDFLAGS=$WARN_CFLAGS
AC_SUBST([WARN_CFLAGS])
--
1.7.7.6
12 years, 7 months
