[libvirt] [PATCHv6 7/8] blockjob: allow for existing files
This copies some of the checks from snapshots regarding testing
when a file already exists. In the process, I noticed snapshots
had hard-to-read logic, as well as a missing sanity check:
REUSE_EXT should require the destination to already be present.
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow REUSE_EXT
flag.
(qemuDomainBlockCopy): Wire up flag, and add some sanity checks.
(qemuDomainSnapshotDiskPrepare): Require destination on REUSE_EXT.
---
v6: no real change from v5
src/qemu/qemu_driver.c | 50 ++-
1 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cebbf46..e3d3280 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9829,8 +9829,13 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm,
virDomainSnapshotDefPtr def,
_("unable to stat for disk %s: %s"),
disk->name, disk->file);
goto cleanup;
+} else if (allow_reuse) {
+virReportSystemError(errno,
+ _("missing existing file for disk %s:
%s"),
+ disk->name, disk->file);
+goto cleanup;
}
-} else if (!(S_ISBLK(st.st_mode) || !st.st_size || allow_reuse)) {
+} else if (!S_ISBLK(st.st_mode) && st.st_size && !allow_reuse) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("external snapshot file for disk %s already "
"exists and is not a block device: %s"),
@@ -11962,9 +11967,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
virDomainDiskDefPtr disk;
int ret = -1;
int idx;
+struct stat st;
/* Preliminaries: find the disk we are editing, sanity checks */
-virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW, -1);
+virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
+ VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);
qemuDriverLock(driver);
virUUIDFormat(dom->uuid, uuidstr);
@@ -12016,12 +12023,42 @@ qemuDomainBlockCopy(virDomainPtr dom, const char
*path,
goto endjob;
}
+/* XXX this is pessimistic; we could use 'query-block' or even
+ * keep track of the backing chain ourselves, rather than assuming
+ * that all non-raw source files have a current backing image */
+if ((flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) &&
+STREQ_NULLABLE(format, "raw") &&
+STRNEQ_NULLABLE(disk->driverType, "raw")) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("raw shallow copy of non-raw disk '%s' not
possible"),
+disk->dst);
+goto endjob;
+}
+
/* Prepare the destination file. */
+if (stat(dest, &st) < 0) {
+if (errno != ENOENT) {
+virReportSystemError(errno, _("unable to stat for disk %s: %s"),
+ disk->dst, dest);
+goto endjob;
+} else if (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) {
+virReportSystemError(errno,
+ _("missing destination file for disk %s: %s"),
+ disk->dst, dest);
+goto endjob;
+}
+} else if (!S_ISBLK(st.st_mode) && st.st_size &&
+ !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("external destination file for disk %s already "
+ "exists and is not a block device: %s"),
+disk->dst, dest);
+goto endjob;
+}
+
/* XXX We also need to add security labeling, lock manager lease,
- * and auditing of those events, as well as to support reuse of
- * existing images, including probing the existing format of an
- * existing image. */
-if (!format)
+ * and auditing of those events. */
+if (!format && !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))
format = disk->driverType;
if ((format && !(disk->mirrorFormat = strdup(format))) ||
!(disk->mirror = strdup(dest))) {
@@ -12060,6 +12097,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char
*path, const char *base,
unsigned long bandwidth, unsigned int flags)
{
virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
+ VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
VIR_DOMAIN_BLOCK_REBASE_COPY |
VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1);
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Adds support to VIR_DOMAIN_CPU_STATS_F_VCPU in qemu_driver.
On Wed, Apr 18, 2012 at 08:43:42AM -0600, Eric Blake wrote:
> On 04/18/2012 05:14 AM, Hu Tao wrote:
> > ---
> > src/qemu/qemu_driver.c | 152
> > ++-
> > src/util/cgroup.c |4 +-
> > tools/virsh.c | 17 --
> > 3 files changed, 150 insertions(+), 23 deletions(-)
>
> Commit message is too sparse. You are mixing a lot of things in one
> patch; personally, I would have moved the virsh.c change into patch 1,
> where you are defining the new API, leaving just the cgroup and
> qemu_driver changes as a single patch to implement the API.
>
> What does the _F_ in VIR_DOMAIN_CPU_STATS_F_VCPU stand for?
>
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 0d3b0bd..165b5f3 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -12377,19 +12377,110 @@ qemuDomainGetTotalcpuStats(virCgroupPtr group,
> > return nparams;
> > }
> >
> > +/* get the cpu time from cpuacct cgroup group, saving
> > + cpu time value in cpu_time. caller is responsible
> > + for freeing memory allocated for cpu_time.
> > + return 0 on success, -1 otherwise */
> > +static int getVcpuPercpuStats(virCgroupPtr group,
> > + unsigned long long **cpu_time,
> > + unsigned int *num)
> > +{
> > +int ret = -1;
> > +unsigned long long *ptime = NULL;
> > +char *buf = NULL;
> > +char *pos;
> > +unsigned long long tmp;
> > +
> > +if (virCgroupGetCpuacctPercpuUsage(group, &buf))
> > +goto error;
> > +
> > +pos = buf;
> > +*num = 0;
> > +while (virStrToLong_ull(pos, &pos, 10, &tmp) == 0)
> > +(*num)++;
> > +
> > +if (*num > 0) {
> > +int i;
> > +
> > +if (VIR_ALLOC_N(ptime, *num) < 0)
> > +goto error;
>
> Missing a virReportOOMError().
>
> > +
> > +pos = buf;
> > +for (i = 0; i < *num; i++)
> > +virStrToLong_ull(pos, &pos, 10, ptime + i);
>
> No error checking?
>
> > +*cpu_time = ptime;
> > +ret = 0;
> > +}
> > +error:
> > +return ret;
>
> Leaks buf. How does the caller know how many entries were allocated
> into *cpu_time?
The number is returned by parameter num.
>
> > +}
> > +
> > +static int getSumVcpuPercpuStats(virCgroupPtr group,
> > + unsigned int nvcpu,
> > + unsigned long long **sum_cpu_time,
> > + unsigned int *num)
> > +{
> > +unsigned long long *cpu_time[nvcpu];
>
> I'm not sure whether use of nvcpu as an array length in a local variable
> declaration is portable.
>
> > +unsigned int ncpu_time[nvcpu];
> > +unsigned int max = 0;
> > +unsigned long long *tmp = NULL;
> > +virCgroupPtr group_vcpu = NULL;
> > +int ret = -1;
> > +int i, j;
> > +
> > +for (i = 0; i < nvcpu; i++) {
> > +ret = virCgroupForVcpu(group, i, &group_vcpu, 0);
> > +if (ret < 0) {
> > +qemuReportError(VIR_ERR_INTERNAL_ERROR,
> > +_("cpuacct parse error"));
> > +goto error;
> > +}
> > +ret = getVcpuPercpuStats(group_vcpu,
> > + &cpu_time[i],
> > + &ncpu_time[i]);
> > +if (ret < 0)
> > +goto error;
> > +if (max < ncpu_time[i])
> > +max = ncpu_time[i];
> > +}
> > +
> > +if (max > 0) {
> > +if (VIR_ALLOC_N(tmp, max) < 0)
> > +goto error;
>
> If this allocation fails...
>
> > +
> > +memset(tmp, 0, sizeof(*tmp) * max);
>
> Useless memset. VIR_ALLOC_N already guarantees zero-initialization.
>
> > +for (i = 0; i < nvcpu; i++) {
> > +for (j = 0; j < ncpu_time[i]; j++)
> > +tmp[j] += cpu_time[i][j];
> > +}
> > +*sum_cpu_time = tmp;
> > +*num = max;
> > +ret = 0;
> > +}
> > +
> > +for (i = 0; i < nvcpu; i++)
> > +VIR_FREE(cpu_time[i]);
> > +
> > +error:
> > +return ret;
>
> ...then this leaks each cpu_time[i]. You need to move the error: label
> up by two lines.
>
> > +}
> > +
> > static int
> > qemuDomainGetPercpuStats(virDomainPtr domain,
> > + virDomainObjPtr vm,
> > virCgroupPtr group,
> > virTypedParameterPtr params,
> > unsigned int nparams,
> > int start_cpu,
> > - unsigned int ncpus)
> > + unsigned int ncpus,
> > + unsigned int flags)
> > {
> > char *map = NULL;
> > int rv = -1;
> > int i, max_id;
> > char *pos;
> > char *buf = NULL;
> > +qemuDomainObjPrivatePtr priv = vm->privateData;
> > virTypedParameterPtr ent;
> > int param_idx;
> >
> > @@ -12425,22 +12516,48 @@ qem
[libvirt] [PATCHv6 4/8] blockjob: support pivot operation on cancel
This is the bare minimum to end a copy job (of course, until a
later patch adds the ability to start a copy job, this patch
doesn't do much in isolation; I've just split the patches to
ease the review).
This patch intentionally avoids SELinux, lock manager, and audit
actions. Also, if libvirtd restarts at the exact moment that a
'drive-reopen' is in flight, the proposed proper way to detect the
outcome of that 'drive-reopen' would be to first pass in a witness
fd with 'getfd', then at libvirtd restart, probe whether that file
is still empty. This patch is enough to test the common case of
success when used correctly, while saving the subtleties of proper
cleanup for worst-case errors for later.
When a mirror job is started, cancelling the job safely reverts back
to the source disk, regardless of whether the destination is in
phase 1 (streaming, in which case the destination is worthless) or
phase 2 (mirroring, in which case the destination is synced up to
the source at the time of the cancel). Our existing code does just
fine in either phase, other than some bookkeeping cleanup; this
implements live block copy, even if qemu 1.1 lacks 'drive-reopen'.
Pivoting the job requires the use of the new 'drive-reopen' command.
Here, failure of the command is potentially catastrophic to the
domain, since the initial qemu implementation rips out the old disk
before attempting to open the new one; qemu will attempt a recovery
path of retrying the reopen on the original source, but if that also
fails, the domain is hosed, with nothing libvirt can do about it.
Ideas for future enhancements via new flags:
If qemu 1.2 ever adds 'drive-reopen' inside 'transaction', then the
problem will no longer exist (a transaction promises not to close
the old file until after the new file is proven to work), at which
point we would add a VIR_DOMAIN_REBASE_COPY_ATOMIC that fails up
front if we detect an older qemu with the risky drive-reopen. We
may also want to add a flag that fails up front if there is no
reopen support at all, rather than waiting until the entire copy
is done only to find out that pivot always fails.
Interesting side note: while snapshot-create --disk-only creates a
copy of the disk at a point in time by moving the domain on to a
new file (the copy is the file now in the just-extended backing
chain), blockjob --abort of a copy job creates a copy of the disk
while keeping the domain on the original file. There may be
potential improvements to the snapshot code to exploit block copy
over multiple disks all at one point in time. And, if
'block-job-cancel' were made part of 'transaction', you could
copy multiple disks at the same point in time without pausing
the domain. This also implies we may want to add a --quiesce flag
to virDomainBlockJobAbort, so that when breaking a mirror (whether
by cancel or pivot), the side of the mirror that we are abandoning
is at least in a stable state with regards to guest I/O.
* src/qemu/qemu_driver.c (qemuDomainBlockJobAbort): Accept new flag.
(qemuDomainBlockPivot): New helper function.
(qemuDomainBlockJobImpl): Implement it.
---
v6: add probe for drive-reopen at pivot time, so that it is
possible to support copy but not pivot with just drive-mirror
src/qemu/qemu_driver.c | 112 +++-
1 files changed, 111 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e1584c6..e562844 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11640,6 +11640,86 @@ cleanup:
return ret;
}
+/* Called while holding the VM job lock, to implement a block job
+ * abort with pivot; this updates the VM definition as appropriate, on
+ * either success or failure (although there are some forms of
+ * catastrophic failure that will leave the VM unusable). */
+static int
+qemuDomainBlockPivot(struct qemud_driver *driver, virDomainObjPtr vm,
+ const char *device, virDomainDiskDefPtr disk)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainBlockJobInfo info;
+
+/* Probe the status, if needed. */
+if (!disk->mirroring) {
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorBlockJob(priv->mon, device, NULL, 0, &info,
+ BLOCK_JOB_INFO, true);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+if (ret < 0)
+goto cleanup;
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+_("domain is not running"));
+goto cleanup;
+}
+if (ret == 1 && info.cur == info.end &&
+info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
+disk->mirroring = true;
+}
+
+if (!disk->mirroring) {
+qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+_("disk '%s' not ready for pivot yet"),
+disk
[libvirt] [PATCHv6 1/8] blockjob: react to active block copy
For now, disk migration via block copy job is not implemented. But
when we do implement it, we have to deal with the fact that qemu does
not provide an easy way to re-start a qemu process with mirroring
still intact (it _might_ be possible by using qemu -S then an
initial 'drive-mirror' with disk reuse before starting the domain,
but that gets hairy). Even something like 'virDomainSave' becomes
hairy, if you realize the implications that 'virDomainRestore' would
be stuck with recreating the same mirror layout.
But if we step back and look at the bigger picture, we realize that
the initial client of live storage migration via disk mirroring is
oVirt, which always uses transient domains, and that if a transient
domain is destroyed while a mirror exists, oVirt can easily restart
the storage migration by creating a new domain that visits just the
source storage, with no loss in data.
We can make life a lot easier by being cowards, and forbidding
certain operations on a domain. This patch guarantees that we
never get in a state where we would have to restart a domain with
a mirroring block copy, by preventing saves, snapshots, hot
unplug of a disk in use, and conversion to a persistent domain
(thankfully, it is still relatively easy to 'virsh undefine' a
running domain to temporarily make it transient, run tests on
'virsh blockcopy', then 'virsh define' to restore the persistence).
The change to qemudDomainDefine looks a bit odd for undoing an
assignment, rather than probing up front to avoid the assignment,
but this is because of how virDomainAssignDef combines both a
lookup and assignment into a single function call.
* src/conf/domain_conf.h (virDomainHasDiskMirror): New prototype.
* src/conf/domain_conf.c (virDomainHasDiskMirror): New function.
* src/libvirt_private.syms (domain_conf.h): Export it.
* src/qemu/qemu_driver.c (qemuDomainSaveInternal)
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainBlockJobImpl, qemudDomainDefine): Prevent dangerous
actions while block copy is already in action.
* src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
---
v6: no real change from v5
src/conf/domain_conf.c | 12
src/conf/domain_conf.h |1 +
src/libvirt_private.syms |1 +
src/qemu/qemu_driver.c | 29 +
src/qemu/qemu_hotplug.c |7 +++
5 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 184ff23..c134362 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7187,6 +7187,18 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const
char *name)
return virDomainDiskRemove(def, i);
}
+/* Return true if VM has at least one disk involved in a current block
+ * copy job (that is, with a element in the disk xml). */
+bool
+virDomainHasDiskMirror(virDomainObjPtr vm)
+{
+int i;
+for (i = 0; i < vm->def->ndisks; i++)
+if (vm->def->disks[i]->mirror)
+return true;
+return false;
+}
+
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
{
if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5aa8fc1..9d74f44 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1954,6 +1954,7 @@ virDomainDiskDefPtr
virDomainDiskRemove(virDomainDefPtr def, size_t i);
virDomainDiskDefPtr
virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
+bool virDomainHasDiskMirror(virDomainObjPtr vm);
int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac);
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a4bd916..9d5a471 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -354,6 +354,7 @@ virDomainGraphicsSpiceZlibCompressionTypeFromString;
virDomainGraphicsSpiceZlibCompressionTypeToString;
virDomainGraphicsTypeFromString;
virDomainGraphicsTypeToString;
+virDomainHasDiskMirror;
virDomainHostdevDefAlloc;
virDomainHostdevDefClear;
virDomainHostdevDefFree;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c3555ca..582eafa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2572,6 +2572,11 @@ qemuDomainSaveInternal(struct qemud_driver *driver,
virDomainPtr dom,
"%s", _("domain is marked for auto destroy"));
goto cleanup;
}
+if (virDomainHasDiskMirror(vm)) {
+qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
+_("domain has active block copy job"));
+goto cleanup;
+}
memset(&header, 0, sizeof(header));
memcpy(header.magic, QEMUD_SAVE_PARTIAL, sizeof(header.magic));
@@ -4964,6 +4969,12 @@ static virDomainPtr qemudDomainDefine(virConnectPtr
conn, const char *xml) {
goto cleanup;
}
def = NULL;
+if (virDomainHasDiskMirror(vm)) {
+qemuRepo
[libvirt] [PATCHv6 3/8] blockjob: return appropriate event and info
Handle the new type of block copy event and info. Of course,
this patch does nothing until a later patch actually allows the
creation/abort of a block copy job. And we'd really love to
have an event without having to poll for the transition between
pull and mirroring, but that will have to wait for qemu.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONHandleBlockJobImpl)
(qemuMonitorJSONGetBlockJobInfoOne): Translate new job type.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Snoop a successful
info query to save effort on a pivot request.
---
v6: no real change from v5
src/qemu/qemu_driver.c |6 ++
src/qemu/qemu_monitor_json.c |4
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 582eafa..e1584c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11712,6 +11712,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char
*path, const char *base,
if (ret < 0)
goto endjob;
+/* Snoop block copy operations, so future cancel operations can
+ * avoid checking if pivot is safe. */
+if (mode == BLOCK_JOB_INFO && ret == 1 && disk->mirror &&
+info->cur == info->end && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
+disk->mirroring = true;
+
/* With synchronous block cancel, we must synthesize an event, and
* we silently ignore the ABORT_ASYNC flag. With asynchronous
* block cancel, the event will come from qemu, but without the
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e0ea505..6bce701 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -797,6 +797,8 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon,
if (STREQ(type_str, "stream"))
type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
+else if (STREQ(type_str, "mirror"))
+type = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
switch ((virConnectDomainEventBlockJobStatus) event) {
case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
@@ -3415,6 +3417,8 @@ static int
qemuMonitorJSONGetBlockJobInfoOne(virJSONValuePtr entry,
}
if (STREQ(type, "stream"))
info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
+else if (STREQ(type, "mirror"))
+info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
else
info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv6 5/8] blockjob: make drive-reopen safer
Since libvirt drops locks between issuing a monitor command and
getting a response, it is possible for libvirtd to be restarted
before getting a response on a drive-reopen command; worse, it is
also possible for the guest to shut itself down during the window
while libvirtd is down, ending the qemu process. A management app
needs to know if the pivot happened (and the destination file
contains guest contents not in the source) or failed (and the source
file contains guest contents not in the destination), but since
the job is finished, 'query-block-jobs' no longer tracks the
status of the job, and if the qemu process itself has disappeared,
even 'query-block' cannot be checked to ask qemu its current state.
An upstream qemu proposal[1] suggested adding an optional witness
fd, passed in by 'getfd', where qemu writes one byte to that file at
the point of a successful pivot, and where restarting libvirtd
would check the contents of the witness file. However, that
requires quite a bit of libvirt work[2], especially since libvirt
does not currently have a way to expose such a witness fd to the
management application; and it is not guaranteed that upstream qemu
will even accept the witness fd solution.
[1] https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01638.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg02293.html
Meanwhile, we can go with a simpler solution; it is not as elegant,
but is something that can be implemented right away without design
complications, and can be enhanced later if future qemu gives us
more power. If we surround 'drive-reopen' with a pause/resume
pair, then we can guarantee that the guest cannot modify either
source or destination files in the window of libvirtd uncertainty,
and the management app is guaranteed that either libvirt knows the
outcome and reported it correctly; or that on libvirtd restart, the
guest will still be paused and that the qemu process cannot have
disappeared due to guest shutdown; and use that as a clue that the
management app must implement recovery protocol, with both source
and destination files still being in sync and with 'query-block'
still being an option as part of that recovery. My testing of
early implementations of 'drive-reopen' show that the pause window
will typically be only a fraction of a second.
* src/qemu/qemu_driver.c (qemuDomainBlockPivot): Pause around
drive-reopen.
(qemuDomainBlockJobImpl): Update caller.
---
v6: new patch
src/qemu/qemu_driver.c | 38 --
1 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e562844..adbc27d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11645,12 +11645,14 @@ cleanup:
* either success or failure (although there are some forms of
* catastrophic failure that will leave the VM unusable). */
static int
-qemuDomainBlockPivot(struct qemud_driver *driver, virDomainObjPtr vm,
+qemuDomainBlockPivot(virConnectPtr conn,
+ struct qemud_driver *driver, virDomainObjPtr vm,
const char *device, virDomainDiskDefPtr disk)
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainBlockJobInfo info;
+bool resume = false;
/* Probe the status, if needed. */
if (!disk->mirroring) {
@@ -11677,6 +11679,30 @@ qemuDomainBlockPivot(struct qemud_driver *driver,
virDomainObjPtr vm,
goto cleanup;
}
+/* Until 'drive-reopen' is part of a 'transaction', we want to
+ * make sure that management apps can tell whether the command
+ * succeeded, even if libvirtd is restarted at the wrong time. To
+ * accomplish that, we pause the guest before drive-reopen, and
+ * resume it only when we know the outcome; if libvirtd restarts,
+ * then management will see the guest still paused, and know that
+ * no guest I/O has caused the source and mirror to diverge.
+ *
+ * XXX once 'drive-reopen' is part of 'transaction', then we don't
+ * need to do this; we will want to add an atomic flag that says
+ * whether to fail or fall back to this method for older qemu. */
+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
+if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE,
+QEMU_ASYNC_JOB_NONE) < 0)
+goto cleanup;
+
+resume = true;
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("guest unexpectedly quit"));
+goto cleanup;
+}
+}
+
/* Attempt the pivot. */
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorDriveReopen(priv->mon, device, disk->mirror,
@@ -11717,6 +11743,14 @@ qemuDomainBlockPivot(struct qemud_driver *driver,
virDomainObjPtr vm,
}
cleanup:
+if (resume && virDomainObjIsActive(vm) &&
+qemuProcessStartC
[libvirt] [PATCHv6 8/8] blockjob: allow mirroring under SELinux
This copies heavily from qemuDomainSnapshotCreateSingleDiskActive(),
in order to set the SELinux label, obtain locking manager lease, and
audit the fact that we hand a new file over to qemu. Alas, releasing
the lease and label at the end of the mirroring is a trickier
prospect (we would have to know the backing chain of both source and
destination, and be sure not to revoke rights to any part of the
chain that is shared), so for now, virDomainBlockJobAbort still
leaves things locked and labeled.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set up labeling.
---
v6: no real change from v8
src/qemu/qemu_driver.c | 69 +--
1 files changed, 60 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e3d3280..e78a73e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11968,6 +11968,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
int ret = -1;
int idx;
struct stat st;
+bool need_unlink = false;
+char *mirror = NULL;
+char *mirrorFormat = NULL;
+char *origsrc = NULL;
+char *origdriver = NULL;
/* Preliminaries: find the disk we are editing, sanity checks */
virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
@@ -12056,29 +12061,75 @@ qemuDomainBlockCopy(virDomainPtr dom, const char
*path,
goto endjob;
}
-/* XXX We also need to add security labeling, lock manager lease,
- * and auditing of those events. */
-if (!format && !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))
-format = disk->driverType;
-if ((format && !(disk->mirrorFormat = strdup(format))) ||
-!(disk->mirror = strdup(dest))) {
+if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) {
+int fd = qemuOpenFile(driver, dest, O_WRONLY | O_TRUNC | O_CREAT,
+ &need_unlink, NULL);
+if (fd < 0)
+goto endjob;
+VIR_FORCE_CLOSE(fd);
+if (!format)
+format = disk->driverType;
+}
+if ((format && !(mirrorFormat = strdup(format))) ||
+!(mirror = strdup(dest))) {
virReportOOMError();
goto endjob;
}
+/* Manipulate disk in place, in a way that can be reverted on
+ * failure, in order to set up labeling and locking. */
+origsrc = disk->src;
+disk->src = (char *) dest;
+origdriver = disk->driverType;
+disk->driverType = (char *) "raw"; /* Don't want to probe backing files */
+
+if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0)
+goto endjob;
+if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def,
+disk) < 0) {
+if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
+VIR_WARN("Unable to release lock on %s", dest);
+goto endjob;
+}
+
/* Actually start the mirroring */
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorDriveMirror(priv->mon, device, dest, format, flags);
+virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0);
if (ret == 0 && bandwidth != 0)
ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
BLOCK_JOB_SPEED_INTERNAL, true);
qemuDomainObjExitMonitorWithDriver(driver, vm);
+if (ret < 0) {
+if (virSecurityManagerRestoreImageLabel(driver->securityManager,
+vm->def, disk) < 0)
+VIR_WARN("Unable to restore security label on %s", dest);
+if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
+VIR_WARN("Unable to release lock on %s", dest);
+goto endjob;
+}
+
+disk->src = origsrc;
+origsrc = NULL;
+disk->driverType = origdriver;
+origdriver = NULL;
+
+/* Update vm in place to match changes. */
+need_unlink = false;
+disk->mirror = mirror;
+disk->mirrorFormat = mirrorFormat;
+mirror = NULL;
+mirrorFormat = NULL;
endjob:
-if (ret < 0) {
-VIR_FREE(disk->mirror);
-VIR_FREE(disk->mirrorFormat);
+if (origsrc) {
+disk->src = origsrc;
+disk->driverType = origdriver;
}
+if (need_unlink && unlink(dest))
+VIR_WARN("unable to unlink just-created %s", dest);
+VIR_FREE(mirror);
+VIR_FREE(mirrorFormat);
if (qemuDomainObjEndJob(driver, vm) == 0) {
vm = NULL;
goto cleanup;
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv6 6/8] blockjob: implement block copy for qemu
Minimal patch to wire up all the pieces in the previous patches
to actually enable a block copy job. By minimal, I mean that
qemu creates the file (that is, no REUSE_EXT flag support yet),
SELinux must be disabled, a lock manager is not informed, and
the audit logs aren't updated. But those will be added as
improvements in future patches.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): New function.
(qemuDomainBlockRebase): Call it when appropriate.
---
v6: no real changes from v5
src/qemu/qemu_driver.c | 124 +++-
1 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index adbc27d..cebbf46 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11950,10 +11950,128 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const
char *path,
}
static int
+qemuDomainBlockCopy(virDomainPtr dom, const char *path,
+const char *dest, const char *format,
+unsigned long bandwidth, unsigned int flags)
+{
+struct qemud_driver *driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+qemuDomainObjPrivatePtr priv;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+char *device = NULL;
+virDomainDiskDefPtr disk;
+int ret = -1;
+int idx;
+
+/* Preliminaries: find the disk we are editing, sanity checks */
+virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW, -1);
+
+qemuDriverLock(driver);
+virUUIDFormat(dom->uuid, uuidstr);
+vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+if (!vm) {
+qemuReportError(VIR_ERR_NO_DOMAIN,
+_("no domain with matching uuid '%s'"), uuidstr);
+goto cleanup;
+}
+
+device = qemuDiskPathToAlias(vm, path, &idx);
+if (!device) {
+goto cleanup;
+}
+disk = vm->def->disks[idx];
+if (disk->mirror) {
+qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+_("disk '%s' already in active block copy job"),
+disk->dst);
+goto cleanup;
+}
+
+priv = vm->privateData;
+/* XXX Should we add a flag that lets the user fail up front if
+ * pivot (QEMU_CAPS_DRIVE_REOPEN) is not supported, rather than
+ * making them wait until the mirroring phase to find out? */
+if (!(qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR) &&
+ qemuCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+_("block copy is not supported with this QEMU
binary"));
+goto cleanup;
+}
+if (vm->persistent) {
+/* XXX if qemu ever lets us start a new domain with mirroring
+ * already active, we can relax this; but for now, the risk of
+ * 'managedsave' due to libvirt-guests means we can't risk
+ * this on persistent domains. */
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+_("domain is not transient"));
+goto cleanup;
+}
+
+if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+_("domain is not running"));
+goto endjob;
+}
+
+/* Prepare the destination file. */
+/* XXX We also need to add security labeling, lock manager lease,
+ * and auditing of those events, as well as to support reuse of
+ * existing images, including probing the existing format of an
+ * existing image. */
+if (!format)
+format = disk->driverType;
+if ((format && !(disk->mirrorFormat = strdup(format))) ||
+!(disk->mirror = strdup(dest))) {
+virReportOOMError();
+goto endjob;
+}
+
+/* Actually start the mirroring */
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorDriveMirror(priv->mon, device, dest, format, flags);
+if (ret == 0 && bandwidth != 0)
+ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
+ BLOCK_JOB_SPEED_INTERNAL, true);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+endjob:
+if (ret < 0) {
+VIR_FREE(disk->mirror);
+VIR_FREE(disk->mirrorFormat);
+}
+if (qemuDomainObjEndJob(driver, vm) == 0) {
+vm = NULL;
+goto cleanup;
+}
+
+cleanup:
+VIR_FREE(device);
+if (vm)
+virDomainObjUnlock(vm);
+qemuDriverUnlock(driver);
+return ret;
+}
+
+static int
qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
unsigned long bandwidth, unsigned int flags)
{
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
+ VIR_DOMAIN_BLOCK_REBASE_COPY |
+ VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1);
+
+if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY)
[libvirt] [PATCHv6 0/8] live block migration
v5: https://www.redhat.com/archives/libvir-list/2012-April/msg00753.html Differences in v6: - rebased on top of accepted patches v5:1-4/23 and latest tree - corresponds to patches v5:7-14/23 - patch 5/8 is new - patch v5:12/23 is dropped; qemu is giving us something better, but I still need to finish writing that patch - patch v5:11/23 comments were incorporated, with better cleanup on error - tweak series to deal with potential for qemu 1.1 to support copy but not pivot - limit series to just the bare minimum for now; patches from v5:15/23 are still on my tree, but not worth submitting until we know more about what qemu will provide I'm posting this more for reference for my efforts to backport this to RHEL 6 where I have tested against a candidate qemu build, and don't this is ready for upstream until we have confirmation that actual patches have gone into at least the qemu block queue for 1.1. Eric Blake (8): blockjob: react to active block copy blockjob: add qemu capabilities related to block jobs blockjob: return appropriate event and info blockjob: support pivot operation on cancel blockjob: make drive-reopen safer blockjob: implement block copy for qemu blockjob: allow for existing files blockjob: allow mirroring under SELinux src/conf/domain_conf.c | 12 ++ src/conf/domain_conf.h |1 + src/libvirt_private.syms |1 + src/qemu/qemu_capabilities.c |3 + src/qemu/qemu_capabilities.h |2 + src/qemu/qemu_driver.c | 396 +- src/qemu/qemu_hotplug.c |7 + src/qemu/qemu_monitor.c | 37 src/qemu/qemu_monitor.h | 11 ++ src/qemu/qemu_monitor_json.c | 67 +++ src/qemu/qemu_monitor_json.h | 18 ++- 11 files changed, 549 insertions(+), 6 deletions(-) -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv6 2/8] blockjob: add qemu capabilities related to block jobs
The new block copy storage migration sequence requires new monitor
commands: adding just 'drive-mirror' allows live copy, and also adding
'drive-reopen' allows live migration. Both commands have been
proposed[1] for qemu 1.1, although as of the writing of this commit
message, it appears that drive-mirror might make it, but drive-reopen
will probably be delayed to qemu 1.2. There is consensus that the
libvirt API virDomainBlockRebase as already committed for 0.9.12 is
flexible enough to expose whatever qemu will eventually let us do,
even if the initial libvirt 0.9.12 driver for qemu can't expose the
full power of the API; also, at least RHEL 6.3 will probably be
backporting an early version of drive-reopen under the name
__com.redhat_drive-reopen, where the difference in naming will let us
cater to any minor differences in the monitor command interface while
still preserving the libvirt API at whatever point upstream qemu
commits to the monitor commands. This patch assumes the bare minimum
needed for live migration, but uses separate feature bits in case
qemu 1.1 ends up giving us only live copy.
[1]https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01630.html
* src/qemu/qemu_capabilities.h (QEMU_CAPS_DRIVE_MIRROR)
(QEMU_CAPS_DRIVE_REOPEN): New bits.
* src/qemu/qemu_capabilities.c (qemuCaps): Name them.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set
them.
(qemuMonitorJSONDiskSnapshot): Fix formatting issues.
(qemuMonitorJSONDriveMirror, qemuMonitorDriveReopen): New functions.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDriveMirror)
(qemuMonitorDriveReopen): Declare them.
* src/qemu/qemu_monitor.c (qemuMonitorDriveMirror)
(qemuMonitorDriveReopen): New passthroughs.
* src/qemu/qemu_monitor.h (qemuMonitorDriveMirror)
(qemuMonitorDriveReopen): Declare them.
---
v6: update commit message to point to newer qemu threads, fix
outdated comment in code
src/qemu/qemu_capabilities.c |3 ++
src/qemu/qemu_capabilities.h |2 +
src/qemu/qemu_monitor.c | 37
src/qemu/qemu_monitor.h | 11 +++
src/qemu/qemu_monitor_json.c | 63 ++
src/qemu/qemu_monitor_json.h | 18 ++-
6 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3d1fb43..c5cb1af 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -161,6 +161,9 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"block-job-async",
"scsi-cd",
"ide-cd",
+ "drive-mirror",
+
+ "drive-reopen", /* 95 */
);
struct qemu_feature_flags {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 7279cdb..5472a44 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -128,6 +128,8 @@ enum qemuCapsFlags {
QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */
QEMU_CAPS_SCSI_CD= 92, /* -device scsi-cd */
QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */
+QEMU_CAPS_DRIVE_MIRROR = 94, /* drive-mirror monitor command */
+QEMU_CAPS_DRIVE_REOPEN = 95, /* drive-reopen monitor command */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2f66c46..25704e7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2685,6 +2685,25 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon,
virJSONValuePtr actions,
return ret;
}
+/* Start a drive-mirror block job. */
+int
+qemuMonitorDriveMirror(qemuMonitorPtr mon,
+ const char *device, const char *file,
+ const char *format, unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s, flags=%x",
+ mon, device, file, NULLSTR(format), flags);
+
+if (mon->json)
+ret = qemuMonitorJSONDriveMirror(mon, device, file, format, flags);
+else
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+_("drive-mirror requires JSON monitor"));
+return ret;
+}
+
/* Use the transaction QMP command to run atomic snapshot commands. */
int
qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
@@ -2701,6 +2720,24 @@ qemuMonitorTransaction(qemuMonitorPtr mon,
virJSONValuePtr actions)
return ret;
}
+/* Use the drive-reopen monitor command. */
+int
+qemuMonitorDriveReopen(qemuMonitorPtr mon, const char *device,
+ const char *file, const char *format)
+{
+int ret = -1;
+
+VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s",
+ mon, device, file, NULLSTR(format));
+
+if (mon->json)
+ret = qemuMonitorJSONDriveReopen(mon, device, file, format);
+else
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+_("dr
Re: [libvirt] [PATCH] Add support for firewalld
On 04/23/2012 05:11 PM, Thomas Woerner wrote: Add support for firewalld * bridge_driver, nwfilter_driver: new dbus filters to get FirewallD1.Reloaded signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1 * iptables, ebtables, nwfilter_ebiptables_driver: use firewall-cmd direct passthrough interface Great! After fixing some problems I at least got the nwfilter part to work and the TCK test suite passed without a problem. I didn't test firewalld shutdown and restart interaction so far. I would like to massage the nwfilter part a little bit more tomorrow and send you a patch you can then merge with this one. Thanks! Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] openvz: read vmguarpages/privvmpages to set memory tunables
---
OpenVZ's memory paramters [1,2] aren't very well supported at the
moment. Let's make a start with privvmpages/vmguarpages using already
available memtune parameters. I hope the mapping looks correct.
Cheers,
-- Guido
[1] http://wiki.openvz.org/UBC_primary_parameters
[2] http://wiki.openvz.org/UBC_secondary_parameters
src/openvz/openvz_conf.c | 103 +++
src/openvz/openvz_driver.c | 200
2 files changed, 303 insertions(+)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 5848ec4..579fcfc 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -423,6 +423,108 @@ error:
}
+/* Parse config values of the form barrier:limit into barrier and limit */
+static int
+openvzParseBarrierAndLimit(const char* value,
+ unsigned long *barrier,
+ unsigned long *limit)
+{
+char *token;
+char *saveptr = NULL;
+char *str = strdup(value);
+
+if (str == NULL) {
+virReportOOMError();
+goto error;
+}
+
+token = strtok_r(str, ":", &saveptr);
+if (token == NULL) {
+goto error;
+} else {
+if (barrier != NULL) {
+if (virStrToLong_ul(token, NULL, 10, barrier))
+goto error;
+}
+}
+token = strtok_r(NULL, ":", &saveptr);
+if (token == NULL) {
+goto error;
+} else {
+if (limit != NULL) {
+if (virStrToLong_ul(token, NULL, 10, limit))
+goto error;
+}
+}
+return 0;
+error:
+VIR_FREE(str);
+return -1;
+}
+
+
+static int
+openvzReadMemConf(virDomainDefPtr def, int veid)
+{
+int ret;
+char *temp = NULL;
+unsigned long barrier, limit;
+const char *param;
+unsigned long kb_per_pages;
+
+kb_per_pages = sysconf(_SC_PAGESIZE) / 1024;
+if (kb_per_pages == -1) {
+openvzError(VIR_ERR_INTERNAL_ERROR,
+_("Can't determine page size"));
+goto error;
+}
+
+/* Memory allocation guarantee */
+param = "VMGUARPAGES";
+ret = openvzReadVPSConfigParam(veid, param, &temp);
+if (ret < 0) {
+openvzError(VIR_ERR_INTERNAL_ERROR,
+_("Could not read '%s' from config for container %d"),
+param, veid);
+goto error;
+} else if (ret > 0) {
+ret = openvzParseBarrierAndLimit(temp, &barrier, NULL);
+if (ret < 0) {
+openvzError(VIR_ERR_INTERNAL_ERROR,
+_("Could not parse barrier of '%s' "
+ "from config for container %d"), param, veid);
+goto error;
+}
+def->mem.min_guarantee = barrier * kb_per_pages;
+}
+
+/* Memory hard and soft limits */
+param = "PRIVVMPAGES";
+ret = openvzReadVPSConfigParam(veid, param, &temp);
+if (ret < 0) {
+openvzError(VIR_ERR_INTERNAL_ERROR,
+_("Could not read '%s' from config for container %d"),
+param, veid);
+goto error;
+} else if (ret > 0) {
+ret = openvzParseBarrierAndLimit(temp, &barrier, &limit);
+if (ret < 0) {
+openvzError(VIR_ERR_INTERNAL_ERROR,
+_("Could not parse barrier and limit of '%s' "
+ "from config for container %d"), param, veid);
+goto error;
+}
+def->mem.soft_limit = barrier * kb_per_pages;
+def->mem.hard_limit = limit * kb_per_pages;
+}
+
+ret = 0;
+error:
+VIR_FREE(temp);
+return ret;
+}
+
+
/* Free all memory associated with a openvz_driver structure */
void
openvzFreeDriver(struct openvz_driver *driver)
@@ -535,6 +637,7 @@ int openvzLoadDomains(struct openvz_driver *driver) {
openvzReadNetworkConf(dom->def, veid);
openvzReadFSConf(dom->def, veid);
+openvzReadMemConf(dom->def, veid);
virUUIDFormat(dom->def->uuid, uuidstr);
if (virHashAddEntry(driver->domains.objs, uuidstr, dom) < 0)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index e8b6915..6f8a6a8 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -54,6 +54,7 @@
#include "nodeinfo.h"
#include "memory.h"
#include "virfile.h"
+#include "virtypedparam.h"
#include "logging.h"
#include "command.h"
#include "viruri.h"
@@ -65,6 +66,8 @@
#define CMDBUF_LEN 1488
#define CMDOP_LEN 288
+#define OPENVZ_NB_MEM_PARAM 3
+
static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid);
static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type);
static int openvzDomainGetMaxVcpus(virDomainPtr dom);
@@ -1631,6 +1634,201 @@ cleanup:
return -1;
}
+
+static int
+openvzDomainGetBarrierLimit(virDomainPtr domain,
+const char *param,
+long *barrier,
+long *limit)
+{
+int status, r
Re: [libvirt] udevadm settle can take too long
Guido Günther wrote:
> Hi,
> On Sun, Apr 22, 2012 at 02:41:54PM -0400, Jim Paris wrote:
> > Hi,
> >
> > http://bugs.debian.org/663931 is a bug I'm hitting, where virt-manager
> > times out on the initial connection to libvirt.
>
> I reassigned the bug back to libvirt. I still wonder what triggers this
> though for some users but not for others?
> Cheers,
> -- Guido
On all of my machines, virt-manager hangs if "udevadm settle" hangs.
You can use the program I posted at that bug report to trigger the
udevadm problem (it can be undone by restarting udev).
Libvirtd only triggers the udevadm problem at startup, through its use
of network namespaces while probing lxc. If anything else generates
uevents after that point, then the udevadm problem usually goes away.
For example, any module loads, hardware events (ejecting a CD, closing
a laptop lid, etc), or bringing up or down network interfaces (which
libvirt would typically do by itself when starting a new domain).
So most users might just avoid it through luck. But if you manually
restart libvirtd right before trying virt-manager, you'll probably see
it too.
Thanks,
-jim
> > The basic problem is that, while checking storage volumes,
> > virt-manager causes libvirt to call "udevadm settle". There's an
> > interaction where libvirt's earlier use of network namespaces (to probe
> > LXC features) had caused some uevents to be sent that get filtered out
> > before they reach udev. This confuses "udevadm settle" a bit, and so
> > it sits there waiting for a 2-3 minute built-in timeout before returning.
> > Eventually libvirtd prints:
> > 2012-04-22 18:22:18.678+: 30503: warning : virKeepAliveTimer:182 : No
> > response from client 0x7feec4003630 after 5 keepalive messages in 30 seconds
> > and virt-manager prints:
> > 2012-04-22 18:22:18.931+: 30647: warning : virKeepAliveSend:128 :
> > Failed to send keepalive response to client 0x25004e0
> > and the connection gets dropped.
> >
> > One workaround could be to specify a shorter timeout when doing the
> > settle. The patch appended below allows virt-manager to work,
> > although the connection still has to wait for the 10 second timeout
> > before it succeeds. I don't know what a better solution would be,
> > though. It seems the udevadm behavior might not be considered a bug
> > from the udev/kernel point of view:
> > https://lkml.org/lkml/2012/4/22/60
> >
> > I'm using Linux 3.2.14 with libvirt 0.9.11. You can trigger the
> > udevadm issue using a program I posted at the Debian bug report link
> > above.
> >
> > -jim
> >
> > >From 17e5b9ebab76acb0d711e8bc308023372fbc4180 Mon Sep 17 00:00:00 2001
> > From: Jim Paris
> > Date: Sun, 22 Apr 2012 14:35:47 -0400
> > Subject: [PATCH] shorten udevadmin settle timeout
> >
> > Otherwise, udevadmin settle can take so long that connections from
> > e.g. virt-manager will get closed.
> > ---
> > src/util/util.c |4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/util/util.c b/src/util/util.c
> > index 6e041d6..dfe458e 100644
> > --- a/src/util/util.c
> > +++ b/src/util/util.c
> > @@ -2593,9 +2593,9 @@ virFileFindMountPoint(const char *type
> > ATTRIBUTE_UNUSED)
> > void virFileWaitForDevices(void)
> > {
> > # ifdef UDEVADM
> > -const char *const settleprog[] = { UDEVADM, "settle", NULL };
> > +const char *const settleprog[] = { UDEVADM, "settle", "--timeout",
> > "10", NULL };
> > # else
> > -const char *const settleprog[] = { UDEVSETTLE, NULL };
> > +const char *const settleprog[] = { UDEVSETTLE, "--timeout", "10", NULL
> > };
> > # endif
> > int exitstatus;
> >
> > --
> > 1.7.7
> >
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
> >
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add support for firewalld
Add support for firewalld
* bridge_driver, nwfilter_driver: new dbus filters to get FirewallD1.Reloaded
signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1
* iptables, ebtables, nwfilter_ebiptables_driver: use firewall-cmd direct
passthrough interface
---
configure.ac |9 ++
src/Makefile.am |8 ++---
src/network/bridge_driver.c | 50 +
src/nwfilter/nwfilter_driver.c| 49
src/nwfilter/nwfilter_ebiptables_driver.c | 27
src/util/ebtables.c | 32 ++
src/util/iptables.c | 25 ---
7 files changed, 192 insertions(+), 8 deletions(-)
diff --git a/configure.ac b/configure.ac
index 89fe818..41d9371 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1191,6 +1191,15 @@ AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" =
"xyes"])
AC_SUBST([POLKIT_CFLAGS])
AC_SUBST([POLKIT_LIBS])
+dnl firewalld
+AC_ARG_WITH([firewalld],
+ AC_HELP_STRING([--with-firewalld], [enable firewalld support]))
+if test "x$with_firewalld" = "xyes" ; then
+ AC_DEFINE_UNQUOTED([HAVE_FIREWALLD], [1], [whether firewalld support is
enabled])
+fi
+AM_CONDITIONAL([HAVE_FIREWALLD], [test "x$with_firewalld" = "xyes"])
+
+
dnl Avahi library
AC_ARG_WITH([avahi],
AC_HELP_STRING([--with-avahi], [use avahi to advertise remote daemon
@<:@default=check@:>@]),
diff --git a/src/Makefile.am b/src/Makefile.am
index e48dfa5..e60a8af 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -941,9 +941,9 @@ noinst_LTLIBRARIES += libvirt_driver_network.la
#libvirt_la_BUILT_LIBADD += libvirt_driver_network.la
endif
libvirt_driver_network_la_CFLAGS = \
- -I$(top_srcdir)/src/conf $(AM_CFLAGS)
+ -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS)
if WITH_DRIVER_MODULES
-libvirt_driver_network_la_LIBADD = ../gnulib/lib/libgnu.la
+libvirt_driver_network_la_LIBADD = ../gnulib/lib/libgnu.la $(DBUS_LIBS)
libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
endif
libvirt_driver_network_la_SOURCES = $(NETWORK_DRIVER_SOURCES)
@@ -1086,9 +1086,9 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_nwfilter.la
noinst_LTLIBRARIES += libvirt_driver_nwfilter.la
endif
libvirt_driver_nwfilter_la_CFLAGS = $(LIBPCAP_CFLAGS) \
- -I$(top_srcdir)/src/conf $(AM_CFLAGS)
+ -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS)
libvirt_driver_nwfilter_la_LDFLAGS = $(LD_AMFLAGS)
-libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS)
+libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) $(DBUS_LIBS)
if WITH_DRIVER_MODULES
libvirt_driver_nwfilter_la_LIBADD += ../gnulib/lib/libgnu.la
libvirt_driver_nwfilter_la_LDFLAGS += -module -avoid-version
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d82212f..094bbae 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -63,6 +63,11 @@
#include "virnetdevbridge.h"
#include "virnetdevtap.h"
+#if HAVE_FIREWALLD
+#include "virdbus.h"
+#include "logging.h"
+#endif
+
#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -253,6 +258,24 @@ networkAutostartConfigs(struct network_driver *driver) {
}
}
+#if HAVE_FIREWALLD
+static DBusHandlerResult
+firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED,
DBusMessage *message, void *user_data) {
+struct network_driver *_driverState = (struct network_driver *) user_data;
+
+if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
+ "NameOwnerChanged") ||
+dbus_message_is_signal(message, "org.fedoraproject.FirewallD1",
+ "Reloaded"))
+{
+VIR_DEBUG("Reload in bridge_driver because of firewalld.");
+networkReloadIptablesRules(_driverState);
+}
+
+return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+#endif
+
/**
* networkStartup:
*
@@ -262,6 +285,9 @@ static int
networkStartup(int privileged) {
uid_t uid = geteuid();
char *base = NULL;
+#ifdef HAVE_FIREWALLD
+DBusConnection *sysbus = NULL;
+#endif
if (VIR_ALLOC(driverState) < 0)
goto error;
@@ -326,6 +352,30 @@ networkStartup(int privileged) {
networkDriverUnlock(driverState);
+#ifdef HAVE_FIREWALLD
+if (!(sysbus = virDBusGetSystemBus())) {
+virErrorPtr err = virGetLastError();
+VIR_WARN("DBus not available, disabling firewalld support in
bridge_driver: %s", err->message);
+} else {
+/* add matches for
+ * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop
+ * Reloaded on org.fedoraproject.FirewallD1 for firewalld reload
+ */
+dbus_bus_add_match(sysbus,
+ "type='signal'"
+ ",interface='"DBUS_IN
Re: [libvirt] udevadm settle can take too long
Hi,
On Sun, Apr 22, 2012 at 02:41:54PM -0400, Jim Paris wrote:
> Hi,
>
> http://bugs.debian.org/663931 is a bug I'm hitting, where virt-manager
> times out on the initial connection to libvirt.
I reassigned the bug back to libvirt. I still wonder what triggers this
though for some users but not for others?
Cheers,
-- Guido
>
> The basic problem is that, while checking storage volumes,
> virt-manager causes libvirt to call "udevadm settle". There's an
> interaction where libvirt's earlier use of network namespaces (to probe
> LXC features) had caused some uevents to be sent that get filtered out
> before they reach udev. This confuses "udevadm settle" a bit, and so
> it sits there waiting for a 2-3 minute built-in timeout before returning.
> Eventually libvirtd prints:
> 2012-04-22 18:22:18.678+: 30503: warning : virKeepAliveTimer:182 : No
> response from client 0x7feec4003630 after 5 keepalive messages in 30 seconds
> and virt-manager prints:
> 2012-04-22 18:22:18.931+: 30647: warning : virKeepAliveSend:128 :
> Failed to send keepalive response to client 0x25004e0
> and the connection gets dropped.
>
> One workaround could be to specify a shorter timeout when doing the
> settle. The patch appended below allows virt-manager to work,
> although the connection still has to wait for the 10 second timeout
> before it succeeds. I don't know what a better solution would be,
> though. It seems the udevadm behavior might not be considered a bug
> from the udev/kernel point of view:
> https://lkml.org/lkml/2012/4/22/60
>
> I'm using Linux 3.2.14 with libvirt 0.9.11. You can trigger the
> udevadm issue using a program I posted at the Debian bug report link
> above.
>
> -jim
>
> >From 17e5b9ebab76acb0d711e8bc308023372fbc4180 Mon Sep 17 00:00:00 2001
> From: Jim Paris
> Date: Sun, 22 Apr 2012 14:35:47 -0400
> Subject: [PATCH] shorten udevadmin settle timeout
>
> Otherwise, udevadmin settle can take so long that connections from
> e.g. virt-manager will get closed.
> ---
> src/util/util.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/util.c b/src/util/util.c
> index 6e041d6..dfe458e 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -2593,9 +2593,9 @@ virFileFindMountPoint(const char *type ATTRIBUTE_UNUSED)
> void virFileWaitForDevices(void)
> {
> # ifdef UDEVADM
> -const char *const settleprog[] = { UDEVADM, "settle", NULL };
> +const char *const settleprog[] = { UDEVADM, "settle", "--timeout", "10",
> NULL };
> # else
> -const char *const settleprog[] = { UDEVSETTLE, NULL };
> +const char *const settleprog[] = { UDEVSETTLE, "--timeout", "10", NULL };
> # endif
> int exitstatus;
>
> --
> 1.7.7
>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vbox: Fix passing an empty IMedium* array to IMachine::Delete
Am 23. April 2012 18:48 schrieb Eric Blake :
> On 04/23/2012 02:29 AM, Jean-Baptiste Rouault wrote:
>> On Sunday 22 April 2012 10:35:59 Matthias Bolte wrote:
>>> vboxArray is not castable to a COM item type. vboxArray is a
>>> wrapper around the XPCOM and MSCOM specific array handling.
>>>
>>> In this case we can avoid passing NULL as an empty array to
>>> IMachine::Delete by passing a dummy IMedium* array with a single
>>> NULL item.
>>> ---
>>>
>>> Jean-Baptiste, I can not reproduce the assertion you mentioned, or
>>> I don't know where to look for it. So could you verify that is patch
>>> avoids this assertion?
>>>
>
>>> + /* XPCOM doesn't like NULL as an array, even when the array size
>>> is 0. + * Instead pass it a dummy array to avoid passing NULL. */
>>> + IMedium *array[] = { NULL };
>>> + machine->vtbl->Delete(machine, 0, array, &progress);
>>> # endif
>>> if (progress != NULL) {
>>> progress->vtbl->WaitForCompletion(progress, -1);
>>
>> The patch is good, I don't get the assertion anymore.
>
> Coupled with that functional test, I give my ACK to the code, as a much
> nicer solution than my temporary build-breaker "fix" via a union hack.
Thanks, pushed.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] openvz: add network interface stats
On Fri, Apr 20, 2012 at 12:01:51PM -0400, Laine Stump wrote:
> On 04/19/2012 06:32 PM, Guido Günther wrote:
> > This will only work for veth devices since venet devices don't have
> > a target element.
>
>
> Well, there is precedent for having stats work on some types of
> interfaces and not on others - type='user' interfaces in qemu also don't
> get stats (as the gnome-boxes guys found out, much to their
> disappointment). (Anyway, my point is that this shouldn't stop us from
> enabling stats for those interfaces we *can* support :-)
>
>
> > ---
> > src/openvz/openvz_driver.c | 52
> >
> > 1 file changed, 52 insertions(+)
> >
> > diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> > index 7ec327d..e8b6915 100644
> > --- a/src/openvz/openvz_driver.c
> > +++ b/src/openvz/openvz_driver.c
> > @@ -57,6 +57,7 @@
> > #include "logging.h"
> > #include "command.h"
> > #include "viruri.h"
> > +#include "stats_linux.h"
> >
> > #define VIR_FROM_THIS VIR_FROM_OPENVZ
> >
> > @@ -1672,6 +1673,56 @@ cleanup:
> > return ret;
> > }
> >
> > +static int
> > +openvzDomainInterfaceStats (virDomainPtr dom,
> > +const char *path,
> > +struct _virDomainInterfaceStats *stats)
> > +{
> > +struct openvz_driver *driver = dom->conn->privateData;
> > +virDomainObjPtr vm;
> > +int i;
> > +int ret = -1;
> > +
> > +openvzDriverLock(driver);
> > +vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> > +openvzDriverUnlock(driver);
> > +
> > +if (!vm) {
> > +char uuidstr[VIR_UUID_STRING_BUFLEN];
> > +virUUIDFormat(dom->uuid, uuidstr);
> > +openvzError(VIR_ERR_NO_DOMAIN,
> > +_("no domain with matching uuid '%s'"), uuidstr);
> > +goto cleanup;
> > +}
> > +
> > +if (!virDomainObjIsActive(vm)) {
> > +openvzError(VIR_ERR_OPERATION_INVALID,
> > +"%s", _("domain is not running"));
> > +goto cleanup;
> > +}
> > +
> > +/* Check the path is one of the domain's network interfaces. */
> > +for (i = 0 ; i < vm->def->nnets ; i++) {
> > +if (vm->def->nets[i]->ifname &&
> > +STREQ (vm->def->nets[i]->ifname, path)) {
> > +ret = 0;
> > +break;
> > +}
> > +}
> > +
> > +if (ret == 0)
> > +ret = linuxDomainInterfaceStats(path, stats);
> > +else
> > +openvzError(VIR_ERR_INVALID_ARG,
> > +_("invalid path, '%s' is not a known interface"),
> > path);
> > +
> > +cleanup:
> > +if (vm)
> > +virDomainObjUnlock(vm);
> > +return ret;
> > +}
> > +
> > +
> > static virDriver openvzDriver = {
> > .no = VIR_DRV_OPENVZ,
> > .name = "OPENVZ",
> > @@ -1717,6 +1768,7 @@ static virDriver openvzDriver = {
> > .domainUndefineFlags = openvzDomainUndefineFlags, /* 0.9.4 */
> > .domainGetAutostart = openvzDomainGetAutostart, /* 0.4.6 */
> > .domainSetAutostart = openvzDomainSetAutostart, /* 0.4.6 */
> > +.domainInterfaceStats = openvzDomainInterfaceStats, /* 0.9.12 */
> > .isEncrypted = openvzIsEncrypted, /* 0.7.3 */
> > .isSecure = openvzIsSecure, /* 0.7.3 */
> > .domainIsActive = openvzDomainIsActive, /* 0.7.3 */
>
> ACK. This is pretty much verbatim what is in qemuDomainInterfaceStats,
> and they're both operating on a host-side veth/tap.
Pushed. Thanks,
-- Guido
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Failure to migrate a guest from libvirt-0.9.10+ to libvirt-0.9.4-
On 04/10/2012 05:36 AM, Daniel P. Berrange wrote: > On Tue, Apr 10, 2012 at 11:58:52AM +0800, Daniel Veillard wrote: >> I realize that we have that behaviour for quite some times but I >> wonder about it, basically we always dump an usb controller on the >> XML domain format: >> >> >> >> even if there is no USB device connected to the machine. The drawback >> I see is that if we try to reuse (or migrate) that domain to an older >> verlion of libvirt without that USB controller support, then we just >> fail to parse the domain, even though we don't need the missing >> capability. The real problem is that libvirt prior to 0.9.5 does not recognize a controller of type 'usb', and since this new element is added during any parse of the domain (rather than just during define), it will be added to the xml that is sent during migrate - if the destination host is < libvirt-0.9.5, the migration will always fail. Note that, prior to 0.9.5, libvirt was automatically adding a usb controller to the qemu commandline anyway, so the hardware being presented to the guest in both cases is identical, and supported by the destination libvirt/qemu. The only problem is that the XML that's now being sent (which would tell the destination libvirt to create a device that it's going to create anyway) is causing an error when libvirt tries to parse the domain xml. > As with downgrading libvirt on a host, migrating a guest to an older > version of libvirt is not something we can really support, since you > run a high risk of breaking QEMU / guest ABI, due to the older libvirt > missing out some hardware device that new libvirt has added. While it's not ideal, migrating a running guest to an older host may be something that is necessary from time to time and, while it's obviously not possible in the case where the older host doesn't support the machine type of the guest, we've already done lots of work to make sure that various versions of libvirt can talk to each other in order to perform a migrations (witness all the different migration protocols currently supported by libvirt). There are a lot of situations where this migration would work properly, if not for this one little problem in the XML (which leads to absolutely *no* change in the hardware actually presented to the guest). Some of us have had offline discussion about this today, and a few suggestions were made. Since all the distros could face this problem, it seems best to try and continue this discussion on the list so that everyone can participate. (Unfortunately, the libvirt version doesn't appear anywhere in the XML nor in the migration protocol, so we can't just make a decision whether or not to include the usb controller based on a version number that's easily available.) Here's some of the ideas that were discussed, and at least one reason for each not being an obvious solution: 1) Require the destination to first upgrade to a libvirt that supports the new XML. While this is a reasonable thing for upstream to say, it may not be so easy for a downstream (distro) version. 2) don't auto-generate when parsing/formatting for the purposes of migration. This is less than ideal because it deprives migration of the advantages provided by an explicit . 3) key off of some specific migration detail of the destination host that is only present in libvirt >= 0.9.5. This seems very tricky. 4) only auto-generate the if there are actually USB devices in the domain that will be affected by its existence. This isn't really useful, because in most cases there is a usb tablet device, and even if there wasn't, it's still desirable to have a usb controller in the XML so that virt-manager can easily change it to USB2.0 (and also for the PCI slot of the usb controller to be explicitly reserved on the config) (In the end it may be that the decision is just that upstream libvirt doesn't go to any pains to support this scenario, and the the various downstream builds need to figure out their own solutions if they want to support it, but I thought I'd at least give it a chance here.) > >> So I wonder if the correct thing isn't to add the USB controller >> only if there is an USB device associated to the domain, then failure >> to migrate on an older libvirt does make sense because we require the >> feature. I would assume that application using the USB support wouldn't >> need to have the USB hub exposed to allow adding an USB device, and once >> the USB device is added then changing the kind of USB hub to select >> a different version does make sense. >> >> In general I'm tempted to not dump in the XML things which are there >> by default and would be automatically added if changed or used in some >> ways. I think there is two advantages to this: >> - keep the XML smaller and somehow more readable >> - avoid portability problems on unsupported but unused constructs >> One drawback I could perceive though is that having defaults values not >> explicitely dumped in th
Re: [libvirt] [PATCH] vbox: Fix passing an empty IMedium* array to IMachine::Delete
On 04/23/2012 02:29 AM, Jean-Baptiste Rouault wrote:
> On Sunday 22 April 2012 10:35:59 Matthias Bolte wrote:
>> vboxArray is not castable to a COM item type. vboxArray is a
>> wrapper around the XPCOM and MSCOM specific array handling.
>>
>> In this case we can avoid passing NULL as an empty array to
>> IMachine::Delete by passing a dummy IMedium* array with a single
>> NULL item.
>> ---
>>
>> Jean-Baptiste, I can not reproduce the assertion you mentioned, or
>> I don't know where to look for it. So could you verify that is patch
>> avoids this assertion?
>>
>> +/* XPCOM doesn't like NULL as an array, even when the array size
>> is 0. + * Instead pass it a dummy array to avoid passing NULL. */
>> +IMedium *array[] = { NULL };
>> +machine->vtbl->Delete(machine, 0, array, &progress);
>> # endif
>> if (progress != NULL) {
>> progress->vtbl->WaitForCompletion(progress, -1);
>
> The patch is good, I don't get the assertion anymore.
Coupled with that functional test, I give my ACK to the code, as a much
nicer solution than my temporary build-breaker "fix" via a union hack.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 04/23] blockjob: add new API flags
On 04/18/2012 08:42 AM, Paolo Bonzini wrote: > Il 18/04/2012 16:18, Jiri Denemark ha scritto: >> However, the question is whether we feel comfortable enough with pushing this >> with the risk that qemu implementation is not upstream yet. I think the API >> is >> sane but the history of pushing something that qemu ended up implementing in >> a >> different way worries me a bit. > > FWIW, the worries on the QEMU side aren't really with the API but with > the implementation. We are not sure of the drive-reopen API, but the > mirroring abstractions (apart from transactions, i.e. patches 19-23) > should be fine. Given the various comments, I've gone ahead and pushed patches 4-6 (the API changes); I'll wait for a bit longer for any further word on the qemu side before pushing any qemu implementation patches, but at this point, I think we're now committed to supporting as much of live storage copy as qemu will let us support at the time of the libvirt 0.9.12 release. I'll probably repost a v6 later today, with only minor tweaks for rebasing the qemu implementation to latest; at this point, any changes I make to the patch series will be limited to a possible rewrite of 12/23 (if upstream will let us call block-job-set-speed prior to block-stream or drive-mirror), a possible split of 8/23 (if we are sure that upstream will take drive-mirror but not drive-reopen in time for qemu 1.1), and adding a new patch 14.5/23 to pause and resume the domain around any block-reopen call (to ensure that we know when the guest has pivoted, even if libvirtd is restarted at an inopportune time, based on whether the guest is still paused). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH] remove original 'util' object in testcases
On 04/23/2012 10:17 PM, Osier Yang wrote: On 2012年04月23日 21:46, Guannan Ren wrote: --- repos/domain/cpu_affinity.py|4 ++-- repos/domain/cpu_topology.py|8 repos/domain/ownership_test.py | 12 ++-- repos/domain/restore.py |8 repos/domain/save.py| 10 +- repos/libvirtd/qemu_hang.py | 12 ++-- repos/libvirtd/restart.py | 12 ++-- repos/libvirtd/upstart.py |6 +++--- repos/network/network_list.py | 12 ++-- repos/remoteAccess/tcp_setup.py |8 repos/remoteAccess/tls_setup.py | 28 ++-- repos/sVirt/domain_nfs_start.py |8 12 files changed, 64 insertions(+), 64 deletions(-) ACK, per the utils class is already destroyed in previous commit, but it's better to explain the reason to destroy it in commit message. Osier Thanks, pushed. Guannan Ren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH] remove original 'util' object in testcases
On 2012年04月23日 21:46, Guannan Ren wrote: --- repos/domain/cpu_affinity.py|4 ++-- repos/domain/cpu_topology.py|8 repos/domain/ownership_test.py | 12 ++-- repos/domain/restore.py |8 repos/domain/save.py| 10 +- repos/libvirtd/qemu_hang.py | 12 ++-- repos/libvirtd/restart.py | 12 ++-- repos/libvirtd/upstart.py |6 +++--- repos/network/network_list.py | 12 ++-- repos/remoteAccess/tcp_setup.py |8 repos/remoteAccess/tls_setup.py | 28 ++-- repos/sVirt/domain_nfs_start.py |8 12 files changed, 64 insertions(+), 64 deletions(-) ACK, per the utils class is already destroyed in previous commit, but it's better to explain the reason to destroy it in commit message. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH] remove original 'util' object in testcases
---
repos/domain/cpu_affinity.py|4 ++--
repos/domain/cpu_topology.py|8
repos/domain/ownership_test.py | 12 ++--
repos/domain/restore.py |8
repos/domain/save.py| 10 +-
repos/libvirtd/qemu_hang.py | 12 ++--
repos/libvirtd/restart.py | 12 ++--
repos/libvirtd/upstart.py |6 +++---
repos/network/network_list.py | 12 ++--
repos/remoteAccess/tcp_setup.py |8
repos/remoteAccess/tls_setup.py | 28 ++--
repos/sVirt/domain_nfs_start.py |8
12 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/repos/domain/cpu_affinity.py b/repos/domain/cpu_affinity.py
index 8b65e2f..ee585e6 100644
--- a/repos/domain/cpu_affinity.py
+++ b/repos/domain/cpu_affinity.py
@@ -38,7 +38,7 @@ def redefine_vcpu_number(domobj, domain_name, vcpu):
return doc.toxml()
-def set_vcpus(util, domobj, domain_name, vcpu):
+def set_vcpus(domobj, domain_name, vcpu):
"""set the value of virtual machine to vcpu offline , then boot up
the virtual machine
"""
@@ -225,7 +225,7 @@ def cpu_affinity(params):
if vcpunum != vcpu:
logger.info("set the vcpu of the guest to %s" % vcpu)
-ret = set_vcpus(util, domobj, domain_name, vcpu)
+ret = set_vcpus(domobj, domain_name, vcpu)
if ret != 0:
return 1
diff --git a/repos/domain/cpu_topology.py b/repos/domain/cpu_topology.py
index 120273a..f0f081c 100644
--- a/repos/domain/cpu_topology.py
+++ b/repos/domain/cpu_topology.py
@@ -86,7 +86,7 @@ def guest_define(domobj, domxml, logger):
return 0
-def guest_start(domobj, guestname, util, logger):
+def guest_start(domobj, guestname, logger):
"""start guest"""
timeout = 600
ip = ''
@@ -121,7 +121,7 @@ def guest_start(domobj, guestname, util, logger):
return 0, ip
def cpu_topology_check(ip, username, password,
- sockets, cores, threads, util, logger):
+ sockets, cores, threads, logger):
"""login the guest, run lscpu command to check the result"""
lscpu = "lscpu"
# sleep for 5 seconds
@@ -190,12 +190,12 @@ def cpu_topology(params):
if guest_define(domobj, domxml, logger):
return 1
-ret, ip = guest_start(domobj, guestname, util, logger)
+ret, ip = guest_start(domobj, guestname, logger)
if ret:
return 1
if cpu_topology_check(ip, username, password,
- sockets, cores, threads, util, logger):
+ sockets, cores, threads, logger):
return 1
return 0
diff --git a/repos/domain/ownership_test.py b/repos/domain/ownership_test.py
index acb56c1..b479708 100644
--- a/repos/domain/ownership_test.py
+++ b/repos/domain/ownership_test.py
@@ -35,7 +35,7 @@ def check_domain_running(conn, guestname, logger):
else:
return 0
-def nfs_setup(util, logger):
+def nfs_setup(logger):
"""setup nfs on localhost
"""
logger.info("set nfs service")
@@ -57,7 +57,7 @@ def nfs_setup(util, logger):
return 0
-def chown_file(util, filepath, logger):
+def chown_file(filepath, logger):
"""touch a file and setting the chown
"""
if os.path.exists(filepath):
@@ -87,7 +87,7 @@ def chown_file(util, filepath, logger):
return 0
-def prepare_env(util, dynamic_ownership, use_nfs, logger):
+def prepare_env(dynamic_ownership, use_nfs, logger):
"""configure dynamic_ownership in /etc/libvirt/qemu.conf,
set chown of the file to save
"""
@@ -126,12 +126,12 @@ def prepare_env(util, dynamic_ownership, use_nfs, logger):
logger.error("wrong use_nfs value")
return 1
-ret = chown_file(util, filepath, logger)
+ret = chown_file(filepath, logger)
if ret:
return 1
if use_nfs == 'enable':
-ret = nfs_setup(util, logger)
+ret = nfs_setup(logger)
if ret:
return 1
@@ -175,7 +175,7 @@ def ownership_test(params):
# set env
logger.info("prepare the environment")
-ret = prepare_env(util, dynamic_ownership, use_nfs, logger)
+ret = prepare_env(dynamic_ownership, use_nfs, logger)
if ret:
logger.error("failed to prepare the environment")
return 1
diff --git a/repos/domain/restore.py b/repos/domain/restore.py
index 4ac68be..3a1a5a9 100644
--- a/repos/domain/restore.py
+++ b/repos/domain/restore.py
@@ -16,7 +16,7 @@ optional_params = {}
def get_guest_ipaddr(*args):
"""Get guest ip address"""
-(guestname, util, logger) = args
+(guestname, logger) = args
mac = utils.get_dom_mac_addr(guestname)
logger.debug("guest mac address: %s" % mac)
@@ -47,10 +47,10 @@ def check_guest_restore(*args):
"""Check restore domain result, if restore domain is successful,
guest status will not be paused and can be ping
"""
-(guestname, domobj, util,
[libvirt] [PATCH V3] nwfilter: Add support for ipset
This patch adds support for the recent ipset iptables extension
to libvirt's nwfilter subsystem. Ipset allows to maintain 'sets'
of IP addresses, ports and other packet parameters and allows for
faster lookup (in the order of O(1) vs. O(n)) and rule evaluation
to achieve higher throughput than what can be achieved with
individual iptables rules.
On the command line iptables supports ipset using
iptables ... -m set --match-set -j ...
where 'ipset name' is the name of a previously created ipset and
flags is a comma-separated list of up to 6 flags. Flags use 'src' and 'dst'
for selecting IP addresses, ports etc. from the source or
destination part of a packet. So a concrete example may look like this:
iptables -A INPUT -m set --match-set test src,src -j ACCEPT
Since ipset management is quite complex, the idea was to leave ipset
management outside of libvirt but still allow users to reference an ipset.
The user would have to make sure the ipset is available once the VM is
started so that the iptables rule(s) referencing the ipset can be created.
Using XML to describe an ipset in an nwfilter rule would then look as
follows:
The two parameters on the command line are also the two distinct XML attributes
'ipset' and 'ipsetflags'.
FYI: Here is the man page for ipset:
https://ipset.netfilter.org/ipset.man.html
Regards,
Stefan
---
v3:
- use DATATYPE_IPSETNAME for the name of the ipset
- adapted documentation for 0.9.12
v2:
- split ipset description into ipset and ipsetflags attribute
- improved on documentation
---
docs/formatnwfilter.html.in | 64 ++
docs/schemas/nwfilter.rng | 23 +
src/conf/nwfilter_conf.c | 136 +-
src/conf/nwfilter_conf.h | 13 ++
src/nwfilter/nwfilter_ebiptables_driver.c | 75 +++-
tests/nwfilterxml2xmlin/ipset-test.xml| 24 +
tests/nwfilterxml2xmlout/ipset-test.xml | 24 +
tests/nwfilterxml2xmltest.c |2
8 files changed, 356 insertions(+), 5 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -183,6 +183,8 @@ static const char dstportstart_str[] = "
static const char dstportend_str[] = "dstportend";
static const char dscp_str[] = "dscp";
static const char state_str[]= "state";
+static const char ipset_str[]= "ipset";
+static const char ipsetflags_str[] = "ipsetflags";
#define SRCMACADDRsrcmacaddr_str
#define SRCMACMASKsrcmacmask_str
@@ -206,6 +208,8 @@ static const char state_str[]= "
#define DSTPORTENDdstportend_str
#define DSCP dscp_str
#define STATE state_str
+#define IPSET ipset_str
+#define IPSETFLAGSipsetflags_str
/**
@@ -980,6 +984,97 @@ tcpFlagsFormatter(virBufferPtr buf,
return true;
}
+static bool
+ipsetValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union data *val,
+ virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+ nwItemDesc *item)
+{
+const char *errmsg = NULL;
+
+if (virStrcpy(item->u.ipset.setname, val->c,
+ sizeof(item->u.ipset.setname)) == NULL) {
+errmsg = _("ipset name is too long");
+goto arg_err_exit;
+}
+
+if (item->u.ipset.setname[strspn(item->u.ipset.setname,
+ VALID_IPSETNAME)] != 0) {
+errmsg = _("ipset name contains invalid characters");
+goto arg_err_exit;
+}
+
+return true;
+
+arg_err_exit:
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ "%s", errmsg);
+return false;
+}
+
+static bool
+ipsetFormatter(virBufferPtr buf,
+ virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+ nwItemDesc *item)
+{
+virBufferAdd(buf, item->u.ipset.setname, -1);
+
+return true;
+}
+
+static bool
+ipsetFlagsValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union data
*val,
+virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED, nwItemDesc
*item)
+{
+const char *errmsg = NULL;
+size_t idx = 0;
+
+item->u.ipset.numFlags = 0;
+item->u.ipset.flags = 0;
+
+errmsg = _("malformed ipset flags");
+
+while (item->u.ipset.numFlags < 6) {
+if (STRCASEEQLEN(&val->c[idx], "src", 3)) {
+item->u.ipset.flags |= (1 << item->u.ipset.numFlags);
+} else if (!STRCASEEQLEN(&val->c[idx], "dst", 3)) {
+goto arg_err_exit;
+}
+item->u.ipset.numFlags++;
+idx += 3;
+if (val->c[idx] != ',')
+break;
+idx++;
+}
+
+if (val->c[idx] != '\0')
+goto arg_err_exit;
+
+return true;
+
+arg_err_exit:
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ "%s", errmsg);
+return false;
+}
+
[libvirt] [PATCH 1/2] [TCK] nwfilter: Adapt test program and cases to recent iptables
Recent iptables fixes a lot of issues with missing spaces and
other information that was previously not reported properly. To make
the test program and test cases work on old and newer installations
of iptables tools, some adjustments need to be made.
Fix a 'file not found error' when running this tool from the shell
directly.
---
scripts/nwfilter/nwfilter2vmtest.sh|6 +++---
scripts/nwfilter/nwfilterxml2fwallout/conntrack-test.fwall | 12 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
===
--- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh
+++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
@@ -9,7 +9,7 @@ VIRSH=virsh
# For each line starting with uri=, remove the prefix and set the hold
# space to the rest of the line. Then at file end, print the hold
# space, which is effectively the last uri= line encountered.
-uri=$(sed -n '/^uri[ ]*=[ ]*/ {
+[ -r "$LIBVIRT_TCK_CONFIG" ] && uri=$(sed -n '/^uri[ ]*=[ ]*/ {
s///
h
}
@@ -147,12 +147,12 @@ checkExpectedOutput() {
break
fi
-diff ${tmpfile} ${tmpfile2} >/dev/null
+diff -w ${tmpfile} ${tmpfile2} >/dev/null
if [ $? -ne 0 ]; then
if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then
echo "FAIL ${xmlfile} : ${cmd}"
-diff ${tmpfile} ${tmpfile2}
+diff -w ${tmpfile} ${tmpfile2}
fi
failctr=$(($failctr + 1))
if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/conntrack-test.fwall
===
--- libvirt-tck.orig/scripts/nwfilter/nwfilterxml2fwallout/conntrack-test.fwall
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/conntrack-test.fwall
@@ -1,18 +1,18 @@
-#iptables -L FI-vnet0 -n
+#iptables -L FI-vnet0 -n | sed 's|#conn/|#conn src/|'
Chain FI-vnet0 (1 references)
target prot opt source destination
-DROP icmp -- 0.0.0.0/00.0.0.0/0 #conn/32 > 1
-DROP tcp -- 0.0.0.0/00.0.0.0/0 #conn/32 > 2
+DROP icmp -- 0.0.0.0/00.0.0.0/0 #conn src/32 > 1
+DROP tcp -- 0.0.0.0/00.0.0.0/0 #conn src/32 > 2
RETURN all -- 0.0.0.0/00.0.0.0/0 state
NEW,ESTABLISHED ctdir REPLY
#iptables -L FO-vnet0 -n
Chain FO-vnet0 (1 references)
target prot opt source destination
ACCEPT all -- 0.0.0.0/00.0.0.0/0 state ESTABLISHED
ctdir ORIGINAL
-#iptables -L HI-vnet0 -n
+#iptables -L HI-vnet0 -n | sed 's|#conn/|#conn src/|'
Chain HI-vnet0 (1 references)
target prot opt source destination
-DROP icmp -- 0.0.0.0/00.0.0.0/0 #conn/32 > 1
-DROP tcp -- 0.0.0.0/00.0.0.0/0 #conn/32 > 2
+DROP icmp -- 0.0.0.0/00.0.0.0/0 #conn src/32 > 1
+DROP tcp -- 0.0.0.0/00.0.0.0/0 #conn src/32 > 2
RETURN all -- 0.0.0.0/00.0.0.0/0 state
NEW,ESTABLISHED ctdir REPLY
#iptables -L libvirt-host-in -n | grep vnet0 | tr -s " "
HI-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-in vnet0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] [TCK] nwfilter: Add test cases for ipset
Add test cases for the ipset extension.
Since ipset may not be available on all system, the first line of the XML
file containing the test filter has been extended with a specially formatted
XML comment containing a command line test for whether the test case can be
run at all. The format of that line is:
If the tests in this line don't succeed, the test case is skipped.
Also add a test case cleaning up the created ipset. Run this test after all
other tests using alphabetical ordering.
---
scripts/nwfilter/nwfilter2vmtest.sh | 36 +++--
scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall| 39 ++
scripts/nwfilter/nwfilterxml2fwallout/zzz-ipset-cleanup.fwall |1
scripts/nwfilter/nwfilterxml2xmlin/ipset-test.xml | 25 ++
scripts/nwfilter/nwfilterxml2xmlin/zzz-ipset-cleanup.xml |5 +
5 files changed, 99 insertions(+), 7 deletions(-)
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2xmlin/ipset-test.xml
===
--- /dev/null
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2xmlin/ipset-test.xml
@@ -0,0 +1,25 @@
+
+
+ 5c6d49af-b071-6127-b4ec-6f8ed4b55335
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
===
--- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh
+++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
@@ -107,6 +107,7 @@ checkExpectedOutput() {
ifname="$3"
flags="$4"
skipregex="$5"
+ skiptest="$6"
regex="s/${ORIG_IFNAME}/${ifname}/g"
tmpdir=$(mktmpdir)
@@ -147,6 +148,18 @@ checkExpectedOutput() {
break
fi
+ if [ -n "${skiptest}" ]; then
+ # treat all skips as passes
+ passctr=$(($passctr + 1))
+ [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && \
+ echo "SKIP ${xmlfile} : ${cmd}"
+ [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \
+ test_result $(($passctr + $failctr)) "" 0
+ [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \
+ tap_pass $(($passctr + $failctr)) "SKIP: ${xmlfile} :
${skiptest}"
+ break
+ fi
+
diff -w ${tmpfile} ${tmpfile2} >/dev/null
if [ $? -ne 0 ]; then
@@ -197,19 +210,27 @@ doTest() {
flags="$5"
testnum="$6"
ctr=0
+ skiptest=""
if [ ! -r "${xmlfile}" ]; then
echo "FAIL : Cannot access filter XML file ${xmlfile}."
return 1
fi
- ${VIRSH} nwfilter-define "${xmlfile}" > /dev/null
+ # Check whether we can run this test at all
+ cmd=`sed -n '1,1 s/^<\!--[ ^I]*#\(.*\)#[ ^I]*-->/\1/p' ${xmlfile}`
+ if [ -n "${cmd}" ]; then
+eval "${cmd}" 2>&1 1>/dev/null
+[ $? -ne 0 ] && skiptest="${cmd}"
+ fi
+
+ [ -z "${skiptest}" ] && ${VIRSH} nwfilter-define "${xmlfile}" > /dev/null
checkExpectedOutput "${xmlfile}" "${fwallfile}" "${vm1name}" "${flags}" \
- ""
+ "" "${skiptest}"
checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \
- "${vm2name}" "${flags}" ""
+ "${vm2name}" "${flags}" "" "${skiptest}"
if [ $(($flags & $FLAG_ATTACH)) -ne 0 ]; then
@@ -234,9 +255,9 @@ EOF
if [ $rc -eq 0 ]; then
checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \
-"${flags}" "(PRE|POST)ROUTING"
+"${flags}" "(PRE|POST)ROUTING" "${skiptest}"
checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \
-"${vm2name}" "${flags}" "(PRE|POST)ROUTING"
+"${vm2name}" "${flags}" "(PRE|POST)ROUTING" "${skiptest}"
msg=`${VIRSH} detach-device "${vm1name}" "${tmpfile}"`
if [ $? -ne 0 ]; then
echo "FAIL: Detach of interface failed."
@@ -246,9 +267,9 @@ EOF
# In case of TAP, run the test anyway so we get to the full number
# of tests
checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \
- "${flags}" "" #"(PRE|POST)ROUTING"
+ "${flags}" "" "${skiptest}" #"(PRE|POST)ROUTING"
checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \
- "${vm2name}" "${flags}" #"(PRE|POST)ROUTING"
+ "${vm2name}" "${flags}" "${skiptest}" #"(PRE|POST)ROUTING"
fi
attachfailctr=$(($attachfailctr + 1))
@@ -357,6 +378,7 @@ createVM() {
+
Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall
===
--- /dev/null
+++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall
@@ -0,0 +1,39 @@
+#iptables -L FI-vnet0 -n
+Chain FI-vnet0 (1 references)
+target prot opt source destination
+RETURN all -- 0.0.0.0/00.0.0.0/0state
NEW,ESTABLISHED ctdir REPLY match-set tck_test src,dst
+RETURN all --
Re: [libvirt] [PATCH] cpu: Improve error reporting on incompatible CPUs
On 04/19/2012 03:45 PM, Jiri Denemark wrote: On Wed, Apr 18, 2012 at 15:19:53 +0200, Peter Krempa wrote: This patch modifies the CPU comparrison function to report the incompatibilities in more detail to ease identification of problems. * src/cpu/cpu.h: cpuGuestData(): Add argument to return detailed error message. * src/cpu/cpu.c: cpuGuestData(): Add passthrough for error argument. * src/cpu/cpu_x86.c x86FeatureNames(): Add function to convert a CPU definition to flag names. x86Compute(): - Add error message parameter - Add macro for reporting detailed error messages. - Improve error reporting. - Simplify calculation of forbidden flags. x86DataIteratorInit(): x86cpuidMatchAny(): Remove functions that are no longer needed. * src/qemu/qemu_command.c: qemuBuildCpuArgStr(): - Modify for new function prototype - Add detailed error reports - Change error code on incompatible processors to VIR_ERR_CONFIG_UNSUPPORTED instead of internal error * tests/cputest.c: cpuTestGuestData(): Modify for new function prototype --- ACK I added the translation macros and removed the semicolon and pushed. Thanks Peter Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH] Fix a bug in the case that clean flag is used
On 04/23/2012 05:23 PM, Osier Yang wrote:
On 2012年04月23日 16:22, Guannan Ren wrote:
When clean flag is used in testcase, the framework
"clean"
reports 'KeyError'.
The bug is caused is by getting mod_case earlier
"mode_case"
than checking whether the 'clean' flag is present or not.
I'd think it doesn't explain the bug well, though I'm
not sure how to give a good explaination too. Perhaps
an example will be good. (It looks to me there is wrong
order of the string manipulation, and thus got a unknown
key?)
Yes, you are right.
The testcases without flag set doesn't be affected.
without 'clean' flag set are not affected.
BTW, could you destroy the "4 leading spaces" in commit
message, though it's fine, but nobody uses it. :-)
ok, thanks
---
src/generator.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/generator.py b/src/generator.py
index 6a46889..2e4ae07 100644
--- a/src/generator.py
+++ b/src/generator.py
@@ -115,11 +115,11 @@ class FuncGen(object):
clean_flag = False
mod_case_func = self.case_name_list[i]
-mod_case = mod_case_func.rsplit(":", 1)[0]
if mod_case_func.endswith(':clean'):
mod_case_func = mod_case_func[:-6]
clean_flag = True
+mod_case = mod_case_func.rsplit(":", 1)[0]
self.fmt.print_start(mod_case, env_logger)
case_params = self.case_params_list[i]
Anyway, it looks right fix, ACK with the nits fixed.
Osier
Thanks and pushed with nits fixed.
Guannan Ren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH] Fix a bug in the case that clean flag is used
On 2012年04月23日 16:22, Guannan Ren wrote:
When clean flag is used in testcase, the framework
"clean"
reports 'KeyError'.
The bug is caused is by getting mod_case earlier
"mode_case"
than checking whether the 'clean' flag is present or not.
I'd think it doesn't explain the bug well, though I'm
not sure how to give a good explaination too. Perhaps
an example will be good. (It looks to me there is wrong
order of the string manipulation, and thus got a unknown
key?)
The testcases without flag set doesn't be affected.
without 'clean' flag set are not affected.
BTW, could you destroy the "4 leading spaces" in commit
message, though it's fine, but nobody uses it. :-)
---
src/generator.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/generator.py b/src/generator.py
index 6a46889..2e4ae07 100644
--- a/src/generator.py
+++ b/src/generator.py
@@ -115,11 +115,11 @@ class FuncGen(object):
clean_flag = False
mod_case_func = self.case_name_list[i]
-mod_case = mod_case_func.rsplit(":", 1)[0]
if mod_case_func.endswith(':clean'):
mod_case_func = mod_case_func[:-6]
clean_flag = True
+mod_case = mod_case_func.rsplit(":", 1)[0]
self.fmt.print_start(mod_case, env_logger)
case_params = self.case_params_list[i]
Anyway, it looks right fix, ACK with the nits fixed.
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vbox: Fix passing an empty IMedium* array to IMachine::Delete
On Sunday 22 April 2012 10:35:59 Matthias Bolte wrote:
> vboxArray is not castable to a COM item type. vboxArray is a
> wrapper around the XPCOM and MSCOM specific array handling.
>
> In this case we can avoid passing NULL as an empty array to
> IMachine::Delete by passing a dummy IMedium* array with a single
> NULL item.
> ---
>
> Jean-Baptiste, I can not reproduce the assertion you mentioned, or
> I don't know where to look for it. So could you verify that is patch
> avoids this assertion?
>
> src/vbox/vbox_tmpl.c |9 -
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 57c18a4..4b0ee2e 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -5294,11 +5294,10 @@ vboxDomainUndefineFlags(virDomainPtr dom, unsigned
> int flags)
>
> ((IMachine_Delete)machine->vtbl->Delete)(machine, &safeArray,
> &progress); # else
> -union {
> -vboxArray array;
> -IMedium *medium;
> -} u = { .array = VBOX_ARRAY_INITIALIZER };
> -machine->vtbl->Delete(machine, 0, &u.medium, &progress);
> +/* XPCOM doesn't like NULL as an array, even when the array size
> is 0. + * Instead pass it a dummy array to avoid passing NULL. */
> +IMedium *array[] = { NULL };
> +machine->vtbl->Delete(machine, 0, array, &progress);
> # endif
> if (progress != NULL) {
> progress->vtbl->WaitForCompletion(progress, -1);
The patch is good, I don't get the assertion anymore.
--
Jean-Baptiste ROUAULT
Ingénieur R&D - diateam : Architectes de l'information
Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCHv2 1/2] sharemod: Add a new file for variable sharing in testcases
On 04/16/2012 04:04 PM, Martin Kletzander wrote:
On 04/16/2012 08:03 AM, Osier Yang wrote:
s/sharing in/sharing among/, but given it's already
pushed. let's live with it.
On 2012年04月10日 21:38, Guannan Ren wrote:
sharedmod.py
---
sharedmod.py | 16
1 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 sharedmod.py
diff --git a/sharedmod.py b/sharedmod.py
new file mode 100644
index 000..8af26d8
--- /dev/null
+++ b/sharedmod.py
@@ -0,0 +1,16 @@
+# This is a module for variable sharing across testcases during
+# running. You have to import it in each of testcases which want
+# to share data. The framwork have already set {'conn': connobj}
+# in libvirtobj dictionary for use in testcases.
+
+# The libvirtobj dictionary is only set and used by framework
Any checking? I.e could testcase r/w it too?
+# in testcases you could use sharedmod.libvirtobj['conn'] to get
+# the connection object in libvirt.py, you need not to close it,
IMHO "need not" should be "should never". How about the
follow up use of connection if it's already closed, reopen one
again?
+# the framework do it.
The framwork have already set {'conn': connobj}
+# in libvirtobj dictionary for use in testcases.
+
The comment above should resides here.
+libvirtobj = {}
+
+# shared variables for customized use in testcases
+# set variable: sharedmod.data['my_test_variable'] = 'test_value'
+# check the variable: sharedmod.data.has_key('my_test_variable')
+# get the varialbe: sharedmod.data.get('my_test_variable',
'test_variable_default_value')
From my p.o.v, "libvirtobj" is a confused name, also "data" is
not enough to tell what the meaning is. How about use
"framework_shares" and "case_shares" instead?
And implement get/set/has functions for both "framework_shares"
and "case_shares", but not access them directly. e.g.
def case_shares_get(key):
pass
def case_shares_set(key, value):
pass
def case_shares_has(key)
pass
Even perhaps a class is good.
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
That's getting closer and closer to what I recommended as a redesign for
the whole test-API. Class would be better, tests shouldn't need to
import the mod every time, there should be checking etc. However I feel
the need to make this stable asap and after some release, we can go
ahead with some major redesigns (or at least that's how I understood our
talk about this with Dave few weeks ago).
Martin
Sorry, I missed the mail thread.
No problem for me to use the class or something in framework.
Use the Class in testcases is not a good idea, it make writing
testcase complex
than the use of function directly.
But, anyway, the common purpose is to ease the work of writing
testcases.
Guannan Ren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH] Fix a bug in the case that clean flag is used
When clean flag is used in testcase, the framework
reports 'KeyError'.
The bug is caused is by getting mod_case earlier
than checking whether the 'clean' flag is present or not.
The testcases without flag set doesn't be affected.
---
src/generator.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/generator.py b/src/generator.py
index 6a46889..2e4ae07 100644
--- a/src/generator.py
+++ b/src/generator.py
@@ -115,11 +115,11 @@ class FuncGen(object):
clean_flag = False
mod_case_func = self.case_name_list[i]
-mod_case = mod_case_func.rsplit(":", 1)[0]
if mod_case_func.endswith(':clean'):
mod_case_func = mod_case_func[:-6]
clean_flag = True
+mod_case = mod_case_func.rsplit(":", 1)[0]
self.fmt.print_start(mod_case, env_logger)
case_params = self.case_params_list[i]
--
1.7.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API 12/17] Substitute 'guest"' with 'domain"'
On 2012年04月23日 15:40, Peter Krempa wrote:
On 04/20/2012 08:46 AM, Osier Yang wrote:
% for i in $(grep 'guest"' * -r | awk -F':' '{print $1}'| uniq); do \
sed -i -e 's/guest"/domain"/g' $i; \
done
This also affects the codes like:
- url = global_parser.get_value("guest", gname + "src")
- dict['kickstart'] = global_parser.get_value("guest", gname + "ks")
+ url = global_parser.get_value("domain", gname + "src")
+ dict['kickstart'] = global_parser.get_value("domain", gname + "ks")
A follow up patch will change the property 'guest' into 'domain'
---
I don't like this very much. "Domain" is a XEN term. IMHO "guest" is
more universal and is a little more specific. (It's harder to
distinguish Xen's Dom0 vs DomU than "host" and "guest")
Yeah, I'd guess it's just caused by XEN is the first hypervisor libvirt
supported, and as a history reason, it's used till now.
Agree that "guest" is better here, but in many other places, "domain"
is better. The problem is there was not a routine for test-API to
follow (even no for libvirt I guess), and the two terms were used
as the coders willing. Actually I found the problem ("guest" is better
in some place) when creating the patch, but I don't have a idea to do it
automatically, doing manually though the project will take much
time.
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API 12/17] Substitute 'guest"' with 'domain"'
On 04/20/2012 08:46 AM, Osier Yang wrote:
% for i in $(grep 'guest"' * -r | awk -F':' '{print $1}'| uniq); do \
sed -i -e 's/guest"/domain"/g' $i; \
done
This also affects the codes like:
-url = global_parser.get_value("guest", gname + "src")
-dict['kickstart'] = global_parser.get_value("guest", gname + "ks")
+url = global_parser.get_value("domain", gname + "src")
+dict['kickstart'] = global_parser.get_value("domain", gname + "ks")
A follow up patch will change the property 'guest' into 'domain'
---
I don't like this very much. "Domain" is a XEN term. IMHO "guest" is
more universal and is a little more specific. (It's harder to
distinguish Xen's Dom0 vs DomU than "host" and "guest")
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
