[libvirt] [PATCH] qemu: Relax -no-shutdown check to [0.14.0, 0.15.0]
The patch that fixes SIGTERM handling with -no-shutdown was taken into 0.15.1 stable release of qemu. --- src/qemu/qemu_capabilities.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8f16a49..2f55000 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1018,9 +1018,9 @@ qemuCapsComputeCmdFlags(const char *help, /* Do not use -no-shutdown if qemu doesn't support it or SIGTERM handling * is most likely buggy when used with -no-shutdown (which applies for qemu - * 0.14.* and 0.15.50) + * 0.14.* and 0.15.0) */ -if (strstr(help, -no-shutdown) (version 14000 || version = 15050)) +if (strstr(help, -no-shutdown) (version 14000 || version 15000)) qemuCapsSet(flags, QEMU_CAPS_NO_SHUTDOWN); /* -- 1.7.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Do not wait if the PCI device is not managed when reattaching
Waiting for qemu-kvm cleaning up the PCI bar(s) mapping with long time
while the device is not managed is just waste of time.
---
src/qemu/qemu_hostdev.c | 15 ---
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index c65f6f5..1fb373e 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -322,19 +322,20 @@ void qemuReattachPciDevice(pciDevice *dev, struct
qemud_driver *driver)
{
int retries = 100;
+if (!pciDeviceGetManaged(dev))
+return;
+
while (pciWaitForDeviceCleanup(dev, kvm_assigned_device)
retries) {
usleep(100*1000);
retries--;
}
-if (pciDeviceGetManaged(dev)) {
-if (pciReAttachDevice(dev, driver-activePciHostdevs) 0) {
-virErrorPtr err = virGetLastError();
-VIR_ERROR(_(Failed to re-attach PCI device: %s),
- err ? err-message : _(unknown error));
-virResetError(err);
-}
+if (pciReAttachDevice(dev, driver-activePciHostdevs) 0) {
+virErrorPtr err = virGetLastError();
+VIR_ERROR(_(Failed to re-attach PCI device: %s),
+ err ? err-message : _(unknown error));
+virResetError(err);
}
}
--
1.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add support for autodestroy of guests to the LXC and UML drivers
From: Daniel P. Berrange berra...@redhat.com
I wrote this months ago and thought I had already submitted/merged
it. Obviously not. I need this for the virt sandbox tools asap.
We recently added support for VIR_DOMAIN_START_AUTODESTROY and
an impl to the QEMU driver. It is very desirable to support in
other drivers, so this adds it to LXC and UML
* src/lxc/lxc_conf.h, src/lxc/lxc_driver.c,
src/uml/uml_conf.h, src/uml/uml_driver.c: Wire up autodestroy
functions
---
src/lxc/lxc_conf.h |5 ++
src/lxc/lxc_driver.c | 140 +--
src/uml/uml_conf.h |6 ++
src/uml/uml_driver.c | 164 +++---
4 files changed, 289 insertions(+), 26 deletions(-)
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index 66aa469..b124330 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -56,6 +56,11 @@ struct __lxc_driver {
int have_netns;
virDomainEventStatePtr domainEventState;
+
+/* Mapping of 'char *uuidstr' - virConnectPtr
+ * of guests which will be automatically killed
+ * when the virConnectPtr is closed*/
+virHashTablePtr autodestroy;
};
int lxcLoadDriverConfig(lxc_driver_t *driver);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c475887..f08e8d1 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -110,6 +110,19 @@ static void lxcDomainEventFlush(int timer, void *opaque);
static void lxcDomainEventQueue(lxc_driver_t *driver,
virDomainEventPtr event);
+static int lxcVmTerminate(lxc_driver_t *driver,
+ virDomainObjPtr vm,
+ virDomainShutoffReason reason);
+static int lxcProcessAutoDestroyInit(lxc_driver_t *driver);
+static void lxcProcessAutoDestroyRun(lxc_driver_t *driver,
+ virConnectPtr conn);
+static void lxcProcessAutoDestroyShutdown(lxc_driver_t *driver);
+static int lxcProcessAutoDestroyAdd(lxc_driver_t *driver,
+virDomainObjPtr vm,
+virConnectPtr conn);
+static int lxcProcessAutoDestroyRemove(lxc_driver_t *driver,
+ virDomainObjPtr vm);
+
static virDrvOpenStatus lxcOpen(virConnectPtr conn,
virConnectAuthPtr auth ATTRIBUTE_UNUSED,
@@ -165,6 +178,7 @@ static int lxcClose(virConnectPtr conn)
lxcDriverLock(driver);
virDomainEventCallbackListRemoveConn(conn,
driver-domainEventState-callbacks);
+lxcProcessAutoDestroyRun(driver, conn);
lxcDriverUnlock(driver);
conn-privateData = NULL;
@@ -1001,6 +1015,104 @@ cleanup:
}
+static int lxcProcessAutoDestroyInit(lxc_driver_t *driver)
+{
+if (!(driver-autodestroy = virHashCreate(5, NULL)))
+return -1;
+
+return 0;
+}
+
+struct lxcProcessAutoDestroyData {
+lxc_driver_t *driver;
+virConnectPtr conn;
+};
+
+static void lxcProcessAutoDestroyDom(void *payload,
+ const void *name,
+ void *opaque)
+{
+struct lxcProcessAutoDestroyData *data = opaque;
+virConnectPtr conn = payload;
+const char *uuidstr = name;
+unsigned char uuid[VIR_UUID_BUFLEN];
+virDomainObjPtr dom;
+virDomainEventPtr event = NULL;
+
+VIR_DEBUG(conn=%p uuidstr=%s thisconn=%p, conn, uuidstr, data-conn);
+
+if (data-conn != conn)
+return;
+
+if (virUUIDParse(uuidstr, uuid) 0) {
+VIR_WARN(Failed to parse %s, uuidstr);
+return;
+}
+
+if (!(dom = virDomainFindByUUID(data-driver-domains,
+uuid))) {
+VIR_DEBUG(No domain object to kill);
+return;
+}
+
+VIR_DEBUG(Killing domain);
+lxcVmTerminate(data-driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED);
+virDomainAuditStop(dom, destroyed);
+event = virDomainEventNewFromObj(dom,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
+
+if (dom !dom-persistent)
+virDomainRemoveInactive(data-driver-domains, dom);
+
+if (dom)
+virDomainObjUnlock(dom);
+if (event)
+lxcDomainEventQueue(data-driver, event);
+virHashRemoveEntry(data-driver-autodestroy, uuidstr);
+}
+
+/*
+ * Precondition: driver is locked
+ */
+static void lxcProcessAutoDestroyRun(lxc_driver_t *driver, virConnectPtr conn)
+{
+struct lxcProcessAutoDestroyData data = {
+driver, conn
+};
+VIR_DEBUG(conn=%p, conn);
+virHashForEach(driver-autodestroy, lxcProcessAutoDestroyDom, data);
+}
+
+static void lxcProcessAutoDestroyShutdown(lxc_driver_t *driver)
+{
+virHashFree(driver-autodestroy);
+}
+
+static int lxcProcessAutoDestroyAdd(lxc_driver_t *driver,
+virDomainObjPtr vm,
+
Re: [libvirt] [PATCH] qemu: Relax -no-shutdown check to [0.14.0, 0.15.0]
On 17.10.2011 12:18, Jiri Denemark wrote: The patch that fixes SIGTERM handling with -no-shutdown was taken into 0.15.1 stable release of qemu. --- src/qemu/qemu_capabilities.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8f16a49..2f55000 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1018,9 +1018,9 @@ qemuCapsComputeCmdFlags(const char *help, /* Do not use -no-shutdown if qemu doesn't support it or SIGTERM handling * is most likely buggy when used with -no-shutdown (which applies for qemu - * 0.14.* and 0.15.50) + * 0.14.* and 0.15.0) */ -if (strstr(help, -no-shutdown) (version 14000 || version = 15050)) +if (strstr(help, -no-shutdown) (version 14000 || version 15000)) qemuCapsSet(flags, QEMU_CAPS_NO_SHUTDOWN); /* I'd just add it's fixed by commit v0.15.0-rc0-885-gd9389b9 in qemu, so ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Do not reattach PCI device used by other domain when shutdown
于 2011年10月17日 09:40, Osier Yang 写道:
于 2011年10月15日 02:53, Eric Blake 写道:
On 10/12/2011 10:05 PM, Osier Yang wrote:
When failing on starting a domain, it tries to reattach all the PCI
devices defined in the domain conf, regardless of whether the devices
are still used by other domain. This will cause the devices to be
deleted
from the list qemu_driver-activePciHostdevs, thus the devices will be
thought as usable even if it's not true. And following commands
nodedev-{reattach,reset} will be successful.
How to reproduce:
1) Define two domains with same PCI device defined in the confs.
2) # virsh start domain1
3) # virsh start domain2
4) # virsh nodedev-reattach $pci_device
This time around, I actually spent time reproducing the bug scenario
on hardware rather than just analyzing by inspection (it took me a
while to figure out why the same machine that used to let me do pci
passthrough was no longer working, until I realized that my hardware
is old enough to be insecure, and I've upgraded software since my
last passthrough test, so the CVE fix from
https://bugzilla.redhat.com/show_bug.cgi?id=71 has to be
intentionally bypassed for me to get passthrough again). With that
testing, I proved that this patch prevents that problem. But, as
written, your patch caused the dreaded An error occurred, but the
cause is unknown.
You will see the device will be reattached to host successfully.
As pciDeviceReattach just check if the device is still used by
other domain via checking if the device is in list
driver-activePciHostdevs,
however, the device is deleted from the list by step 2).
This patch is to prohibit the bug by:
1) Prohibit a domain starting or device attachment right at
preparation period (qemuPrepareHostdevPCIDevices) if the
device is in list driver-activePciHostdevs, which means
it's used by other domain.
2) Introduces a new field for struct _pciDevice, (const char
*used_by),
it will be set as the domain name at preparation period,
(qemuPrepareHostdevPCIDevices). Thus we can prohibit deleting
the device from driver-activePciHostdevs if it's still used by
other domain when stopping the domain process.
@@ -111,7 +112,7 @@ int qemuPrepareHostdevPCIDevices(struct
qemud_driver *driver,
if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs)))
return -1;
-/* We have to use 4 loops here. *All* devices must
+/* We have to use 6 loops here. *All* devices must
* be detached before we reset any of them, because
* in some cases you have to reset the whole PCI,
* which impacts all devices on it. Also, all devices
I added further loop labels.
+/* The device is in use by other active domain if
+ * the dev is in list driver-activePciHostdevs.
+ */
+if (!pciDeviceIsAssignable(dev, !driver-relaxedACS) ||
+pciDeviceListFind(driver-activePciHostdevs, dev))
+goto cleanup;
+}
Here's where the bad message comes in - you jump to cleanup without
ever emitting an error message. Not to mention now that we have the
new used_by field, the message could actually be informative :) With
my changes below, I get the much nicer:
Error starting domain: Requested operation is not valid: PCI device
:0a:0a.0 is in use by domain dom
Oh, right, thanks for the fixing, :)
@@ -277,6 +298,18 @@ void qemuDomainReAttachHostdevDevices(struct
qemud_driver *driver,
for (i = 0; i pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i);
+pciDevice *activeDev = NULL;
+const char *used_by = NULL;
+
+/* Never delete the dev from list driver-activePciHostdevs
+ * if it's used by other domain.
+ */
+activeDev = pciDeviceListFind(driver-activePciHostdevs, dev);
+if (activeDev
+(used_by = pciDeviceGetUsedBy(activeDev))
+STRNEQ(name, used_by))
+continue;
used_by is kept as it might be NULL.
In that case, use STRNEQ_NULLABLE. But you are right - across
libvirtd restarts, we lose track of which domain owns the device - so
I did indeed reproduce a case of NULL. Perhaps food for a later
patch (when reloading domains, cycle through all hostdevs in use by
active domains to repopulate the used_by fields). But not a
show-stopper to getting this bug fix in now.
Yes, I didn't realize this, we need further patch.
After thinking a while, even we set the used_by during domains reloading,
the other attrs (dev-unbind_from_stub, dev-reprobe, dev-remove_slot)
of the PCI dev still will be lost. We can't known what values should be
for these attrs as generally they are initialized when do preparation,
however the preparations are already passed, and the devices are using
by the running domains. And thus the device won't be bound to the
original driver (if it had) correctly, or mistakenly bound to
[libvirt] [libvirt-glib] Remove vir- prefix from signals
gobject signals are generally not namespaced this way, removing this prefix makes things look a bit nicer. --- libvirt-gobject/libvirt-gobject-connection.c | 50 +- libvirt-gobject/libvirt-gobject-domain.c | 10 +++--- libvirt-gobject/libvirt-gobject-manager.c|8 ++-- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 4832908..8813e96 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -170,7 +170,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); -signals[VIR_CONNECTION_OPENED] = g_signal_new(vir-connection-opened, +signals[VIR_CONNECTION_OPENED] = g_signal_new(connection-opened, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_connection_opened), @@ -179,7 +179,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) G_TYPE_NONE, 0); -signals[VIR_CONNECTION_CLOSED] = g_signal_new(vir-connection-closed, +signals[VIR_CONNECTION_CLOSED] = g_signal_new(connection-closed, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_connection_closed), @@ -188,7 +188,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) G_TYPE_NONE, 0); -signals[VIR_DOMAIN_ADDED] = g_signal_new(vir-domain-added, +signals[VIR_DOMAIN_ADDED] = g_signal_new(domain-added, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_domain_added), @@ -198,7 +198,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) 1, G_TYPE_OBJECT); -signals[VIR_DOMAIN_REMOVED] = g_signal_new(vir-domain-removed, +signals[VIR_DOMAIN_REMOVED] = g_signal_new(domain-removed, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_domain_removed), @@ -282,7 +282,7 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) g_signal_emit(gconn, signals[VIR_DOMAIN_ADDED], 0, gdom); else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) -g_signal_emit_by_name(gdom, vir-updated); +g_signal_emit_by_name(gdom, updated); else g_warn_if_reached(); break; @@ -301,60 +301,60 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, case VIR_DOMAIN_EVENT_STARTED: if (detail == VIR_DOMAIN_EVENT_STARTED_BOOTED) -g_signal_emit_by_name(gdom, vir-started::booted); +g_signal_emit_by_name(gdom, started::booted); else if (detail == VIR_DOMAIN_EVENT_STARTED_MIGRATED) -g_signal_emit_by_name(gdom, vir-started::migrated); +g_signal_emit_by_name(gdom, started::migrated); else if (detail == VIR_DOMAIN_EVENT_STARTED_RESTORED) -g_signal_emit_by_name(gdom, vir-started::restored); +g_signal_emit_by_name(gdom, started::restored); else if (detail == VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT) -g_signal_emit_by_name(gdom, vir-started::from-snapshot); +g_signal_emit_by_name(gdom, started::from-snapshot); else g_warn_if_reached(); break; case VIR_DOMAIN_EVENT_SUSPENDED: if (detail == VIR_DOMAIN_EVENT_SUSPENDED_PAUSED) -g_signal_emit_by_name(gdom, vir-suspended::paused); +g_signal_emit_by_name(gdom, suspended::paused); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED) -g_signal_emit_by_name(gdom, vir-suspended::migrated); +g_signal_emit_by_name(gdom, suspended::migrated); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_IOERROR) -g_signal_emit_by_name(gdom, vir-suspended::ioerror); +g_signal_emit_by_name(gdom, suspended::ioerror); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG) -g_signal_emit_by_name(gdom, vir-suspended::watchdog); +g_signal_emit_by_name(gdom, suspended::watchdog); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_RESTORED) -g_signal_emit_by_name(gdom, vir-suspended::restored); +
Re: [libvirt] [PATCHv2 01/13] virbuf: fix const-correctness
On 09/30/2011 12:22 AM, Eric Blake wrote:
Although the compiler wasn't complaining (since it was the pointer,
rather than what was being pointed to, that was actually const), it
looks quite suspicious to call a function with an argument labeled
const when the nature of the pointer (virBufferPtr) is hidden behind
a typedef. Dropping const makes the function declarations easier
to read.
* src/util/buf.h: Drop const from all functions that modify buffer
argument.
* src/util/buf.c (virBufferSetError, virBufferAdd)
(virBufferContentAndReset, virBufferFreeAndReset)
(virBufferAsprintf, virBufferVasprintf, virBufferEscapeString)
(virBufferEscapeSexpr): Fix fallout.
---
src/util/buf.c | 28 ++--
src/util/buf.h | 22 --
2 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/src/util/buf.c b/src/util/buf.c
index 5002486..c737696 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -39,7 +39,7 @@ struct _virBuffer {
* freeing the content and setting the error flag.
*/
static void
-virBufferSetError(const virBufferPtr buf)
+virBufferSetError(virBufferPtr buf)
{
VIR_FREE(buf-content);
buf-size = 0;
@@ -113,7 +113,7 @@ virBufferAdd(const virBufferPtr buf, const char *str, int
len)
Maybe a typo here?The virBufferPtr still remains const, but the
declaration of this function in the buf.h shows this const is removed, too.
/**
* virBufferAddChar:
- * @buf: the buffer to add to
+ * @buf: the buffer to append to
* @c: the character to add
*
* Add a single character 'c' to a buffer.
@@ -150,7 +150,7 @@ virBufferAddChar (virBufferPtr buf, char c)
* Returns the buffer content or NULL in case of error.
*/
char *
-virBufferContentAndReset(const virBufferPtr buf)
+virBufferContentAndReset(virBufferPtr buf)
{
char *str;
if (buf == NULL)
@@ -172,7 +172,7 @@ virBufferContentAndReset(const virBufferPtr buf)
*
* Frees the buffer content and resets the buffer structure.
*/
-void virBufferFreeAndReset(const virBufferPtr buf)
+void virBufferFreeAndReset(virBufferPtr buf)
{
char *str = virBufferContentAndReset(buf);
@@ -214,14 +214,14 @@ virBufferUse(const virBufferPtr buf)
/**
* virBufferAsprintf:
- * @buf: the buffer to dump
+ * @buf: the buffer to append to
* @format: the format
* @...: the variable list of arguments
*
* Do a formatted print to an XML buffer.
*/
void
-virBufferAsprintf(const virBufferPtr buf, const char *format, ...)
+virBufferAsprintf(virBufferPtr buf, const char *format, ...)
{
va_list argptr;
va_start(argptr, format);
@@ -238,7 +238,7 @@ virBufferAsprintf(const virBufferPtr buf, const char
*format, ...)
* Do a formatted print to an XML buffer.
*/
void
-virBufferVasprintf(const virBufferPtr buf, const char *format, va_list argptr)
+virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr)
{
int size, count, grow_size;
va_list copy;
@@ -285,7 +285,7 @@ virBufferVasprintf(const virBufferPtr buf, const char
*format, va_list argptr)
/**
* virBufferEscapeString:
- * @buf: the buffer to dump
+ * @buf: the buffer to append to
* @format: a printf like format string but with only one %s parameter
* @str: the string argument which need to be escaped
*
@@ -293,7 +293,7 @@ virBufferVasprintf(const virBufferPtr buf, const char
*format, va_list argptr)
* is escaped to avoid generating a not well-formed XML instance.
*/
void
-virBufferEscapeString(const virBufferPtr buf, const char *format, const char
*str)
+virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)
{
int len;
char *escaped, *out;
@@ -370,7 +370,7 @@ virBufferEscapeString(const virBufferPtr buf, const char
*format, const char *st
/**
* virBufferEscapeSexpr:
- * @buf: the buffer to dump
+ * @buf: the buffer to append to
* @format: a printf like format string but with only one %s parameter
* @str: the string argument which need to be escaped
*
@@ -379,7 +379,7 @@ virBufferEscapeString(const virBufferPtr buf, const char
*format, const char *st
* doesn't fully escape the sexpr, just enough for our code to work.
*/
void
-virBufferEscapeSexpr(const virBufferPtr buf,
+virBufferEscapeSexpr(virBufferPtr buf,
const char *format,
const char *str)
{
@@ -426,7 +426,7 @@ virBufferEscapeSexpr(const virBufferPtr buf,
/**
* virBufferURIEncodeString:
- * @buf: the buffer to append to
+ * @buf: the buffer to append to
* @str: the string argument which will be URI-encoded
*
* Append the string to the buffer. The string will be URI-encoded
@@ -434,7 +434,7 @@ virBufferEscapeSexpr(const virBufferPtr buf,
* with '%xx' hex sequences).
*/
void
-virBufferURIEncodeString (virBufferPtr buf, const char *str)
+virBufferURIEncodeString(virBufferPtr buf, const char *str)
{
int grow_size = 0;
const char *p;
@@
Re: [libvirt] [libvirt-glib] Remove vir- prefix from signals
On Mon, Oct 17, 2011 at 01:32:58PM +0200, Christophe Fergeau wrote: gobject signals are generally not namespaced this way, removing this prefix makes things look a bit nicer. --- libvirt-gobject/libvirt-gobject-connection.c | 50 +- libvirt-gobject/libvirt-gobject-domain.c | 10 +++--- libvirt-gobject/libvirt-gobject-manager.c|8 ++-- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 4832908..8813e96 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -170,7 +170,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); -signals[VIR_CONNECTION_OPENED] = g_signal_new(vir-connection-opened, +signals[VIR_CONNECTION_OPENED] = g_signal_new(connection-opened, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_connection_opened), @@ -179,7 +179,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) G_TYPE_NONE, 0); -signals[VIR_CONNECTION_CLOSED] = g_signal_new(vir-connection-closed, +signals[VIR_CONNECTION_CLOSED] = g_signal_new(connection-closed, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_connection_closed), @@ -188,7 +188,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) G_TYPE_NONE, 0); -signals[VIR_DOMAIN_ADDED] = g_signal_new(vir-domain-added, +signals[VIR_DOMAIN_ADDED] = g_signal_new(domain-added, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_domain_added), @@ -198,7 +198,7 @@ static void gvir_connection_class_init(GVirConnectionClass *klass) 1, G_TYPE_OBJECT); -signals[VIR_DOMAIN_REMOVED] = g_signal_new(vir-domain-removed, +signals[VIR_DOMAIN_REMOVED] = g_signal_new(domain-removed, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GVirConnectionClass, vir_domain_removed), @@ -282,7 +282,7 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) g_signal_emit(gconn, signals[VIR_DOMAIN_ADDED], 0, gdom); else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) -g_signal_emit_by_name(gdom, vir-updated); +g_signal_emit_by_name(gdom, updated); else g_warn_if_reached(); break; @@ -301,60 +301,60 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, case VIR_DOMAIN_EVENT_STARTED: if (detail == VIR_DOMAIN_EVENT_STARTED_BOOTED) -g_signal_emit_by_name(gdom, vir-started::booted); +g_signal_emit_by_name(gdom, started::booted); else if (detail == VIR_DOMAIN_EVENT_STARTED_MIGRATED) -g_signal_emit_by_name(gdom, vir-started::migrated); +g_signal_emit_by_name(gdom, started::migrated); else if (detail == VIR_DOMAIN_EVENT_STARTED_RESTORED) -g_signal_emit_by_name(gdom, vir-started::restored); +g_signal_emit_by_name(gdom, started::restored); else if (detail == VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT) -g_signal_emit_by_name(gdom, vir-started::from-snapshot); +g_signal_emit_by_name(gdom, started::from-snapshot); else g_warn_if_reached(); break; case VIR_DOMAIN_EVENT_SUSPENDED: if (detail == VIR_DOMAIN_EVENT_SUSPENDED_PAUSED) -g_signal_emit_by_name(gdom, vir-suspended::paused); +g_signal_emit_by_name(gdom, suspended::paused); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED) -g_signal_emit_by_name(gdom, vir-suspended::migrated); +g_signal_emit_by_name(gdom, suspended::migrated); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_IOERROR) -g_signal_emit_by_name(gdom, vir-suspended::ioerror); +g_signal_emit_by_name(gdom, suspended::ioerror); else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG) -g_signal_emit_by_name(gdom, vir-suspended::watchdog); +g_signal_emit_by_name(gdom,
Re: [libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt
Quoting Eli Qiao (ta...@linux.vnet.ibm.com): hi Serge : Thanks for taking a look. I checked the code , only in lxc_controller.c call virFileOpenTtyAt(). I didn't test the path, but my suggestion is that modify the utility function in /src/util/util.c instead of adding a new function. But virFileOpenTtyAt() is called by virFileOpenTty() in the same file. So we really do want a new function using its own versions of grantpt+unlockpt, because I think that, when we can, we want to continue using the glibc versions. So the safe approach seemed to me to be: put the container-specific code into src/lxc/lxc_controller.c, then (in a separate patch) just fold virFileOpenTtyAt(), straight into virFileOpenTty(). If you agree, I'll post a new version incorporating your other feedback, especially the garish use of alloca :) (If you disagree, please feel free to send your own patch - I'm in no way attached to having my version included, I mainly wanted to point out the bug) thanks, -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [BUG, RFC] Python generator: missing error_codes in generated libvirError exceptions
Hello,
I just encountered a situation, where a KVM instance was reported as no
state in virsh. This broke some code of mine, which was
using lookupByUUID() to lookup several VMs. The Python code did raise a
libvirt.libvirtError('virDomainLookupByUUID() failed',), where all
get_error_*() functions return None. This is because the generated libvirt.py
wrapper just raises libvirtError('%s() failed'), which makes deteting the
exact fault harder. My current code explicitly checks for get_error_code() ==
VIR_ERR_NO_DOMAIN.
On further investigation I also noticed that qemudDomainLookupByUUID() uses
qemuReportError() to reports the error condition, which doesn't associate
that error condition with the connection.
What's the most appropriate way to fix this:
1. Assume lookupBy*() only will ever report an invalid lookup key and catch
all libvirtErrors?
2. Parse the exception string?
3. Extend the generator to add additional information from annotations?
4. Replace the generated code with hand-extended code.
Any ideas? Thanks for your input.
Sincerely
Philipp
--
Philipp Hahn Open Source Software Engineer h...@univention.de
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/
signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Console coruption with two or more clients series
I rebased this series for current head. To fetch this series from my repo conviniently: git checkout -b console_corruption 1afcfbdda0cac112faa61f74ec943e46aa43f2f5 git pull git://aeon.pipo.sk/libvirt.git console_corruption Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt
On 10/17/2011 07:26 AM, Serge E. Hallyn wrote: Quoting Eli Qiao (ta...@linux.vnet.ibm.com): hi Serge : Thanks for taking a look. I checked the code , only in lxc_controller.c call virFileOpenTtyAt(). I didn't test the path, but my suggestion is that modify the utility function in /src/util/util.c instead of adding a new function. But virFileOpenTtyAt() is called by virFileOpenTty() in the same file. So we really do want a new function using its own versions of grantpt+unlockpt, because I think that, when we can, we want to continue using the glibc versions. So the safe approach seemed to me to be: put the container-specific code into src/lxc/lxc_controller.c, then (in a separate patch) just fold virFileOpenTtyAt(), straight into virFileOpenTty(). Correct - my intent was that we have: src/util/util.c: virFileOpenTty() - generally useful by any driver, uses glibc, and also should use posix_openpt rather than than open(/dev/ptmx) for portability (gnulib provides posix_openpt). Completely ditch virFileOpenTtyAt() by inlining it back to virFileOpenTty(), and since no one could use what virFileOpenTtyAt() claimed to provide except in the portable case where virFileOpenTty() was sufficient. src/lxc/lxc_controller.c - LXC-specific function that uses ioctl() instead of glibc for opening a private tty. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V1 2/6] Introduce an internal priority for chains
For better handling of the sorting of chains introduce an internally used
priority. Use a lookup table to store the priorities. For now their actual
values do not matter just that the values cause the chains to be properly
sorted through changes in the following patches.
---
src/conf/nwfilter_conf.c | 19 +++
src/conf/nwfilter_conf.h |3 +++
src/nwfilter/nwfilter_ebiptables_driver.c |4
src/nwfilter/nwfilter_ebiptables_driver.h |1 +
4 files changed, 27 insertions(+)
Index: libvirt-acl/src/conf/nwfilter_conf.h
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -355,6 +355,7 @@ enum virNWFilterEbtablesTableType {
};
+# define MIN_RULE_PRIORITY 0
# define MAX_RULE_PRIORITY 1000
enum virNWFilterRuleFlags {
@@ -434,6 +435,7 @@ enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_LAST,
};
+typedef int32_t virNWFilterChainPriority;
typedef struct _virNWFilterDef virNWFilterDef;
typedef virNWFilterDef *virNWFilterDefPtr;
@@ -443,6 +445,7 @@ struct _virNWFilterDef {
unsigned char uuid[VIR_UUID_BUFLEN];
int chainsuffix; /*enum virNWFilterChainSuffixType */
+virNWFilterChainPriority chainPriority;
int nentries;
virNWFilterEntryPtr *filterEntries;
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -120,6 +120,20 @@ struct int_map {
#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
#define INTMAP_ENTRY_LAST { .val = NULL }
+#define NWFILTER_ROOT_FILTER_PRI 0
+#define NWFILTER_IPV4_FILTER_PRI 200
+#define NWFILTER_IPV6_FILTER_PRI 400
+#define NWFILTER_ARP_FILTER_PRI 600
+#define NWFILTER_RARP_FILTER_PRI 800
+
+static const struct int_map chain_priorities[] = {
+INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, root),
+INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, ipv4),
+INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, ipv6),
+INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , arp ),
+INTMAP_ENTRY(NWFILTER_RARP_FILTER_PRI, rarp),
+INTMAP_ENTRY_LAST,
+};
/*
* only one filter update allowed
@@ -2026,6 +2040,11 @@ virNWFilterDefParseXML(xmlXPathContextPt
_(unknown chain suffix '%s'), chain);
goto cleanup;
}
+/* assign an implicit priority -- support XML attribute later */
+if (intMapGetByString(chain_priorities, chain, 0,
+ ret-chainPriority) == false) {
+ret-chainPriority = (MAX_RULE_PRIORITY + MIN_RULE_PRIORITY) / 2;
+}
}
uuid = virXPathString(string(./uuid), ctxt);
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h
@@ -36,6 +36,7 @@ typedef ebiptablesRuleInst *ebiptablesRu
struct _ebiptablesRuleInst {
char *commandTemplate;
enum virNWFilterChainSuffixType neededProtocolChain;
+virNWFilterChainPriority chainPriority;
char chainprefix;/* I for incoming, O for outgoing */
unsigned int priority;
enum RuleType ruleType;
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -327,6 +327,7 @@ static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
enum virNWFilterChainSuffixType neededChain,
+ virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
enum RuleType ruleType)
@@ -340,6 +341,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
inst-commandTemplate = commandTemplate;
inst-neededProtocolChain = neededChain;
+inst-chainPriority = chainPriority;
inst-chainprefix = chainprefix;
inst-priority = priority;
inst-ruleType = ruleType;
@@ -1588,6 +1590,7 @@ _iptablesCreateRuleInstance(int directio
return ebiptablesAddRuleInst(res,
virBufferContentAndReset(final),
nwfilter-chainsuffix,
+ nwfilter-chainPriority,
'\0',
rule-priority,
(isIPv6) ? RT_IP6TABLES : RT_IPTABLES);
@@ -2337,6 +2340,7 @@ ebtablesCreateRuleInstance(char chainPre
return ebiptablesAddRuleInst(res,
virBufferContentAndReset(buf),
Re: [libvirt] [PATCHv2 01/13] virbuf: fix const-correctness
On 10/17/2011 05:20 AM, Hai Dong Li wrote:
On 09/30/2011 12:22 AM, Eric Blake wrote:
Although the compiler wasn't complaining (since it was the pointer,
rather than what was being pointed to, that was actually const), it
looks quite suspicious to call a function with an argument labeled
const when the nature of the pointer (virBufferPtr) is hidden behind
a typedef. Dropping const makes the function declarations easier
to read.
+++ b/src/util/buf.c
@@ -39,7 +39,7 @@ struct _virBuffer {
* freeing the content and setting the error flag.
*/
static void
-virBufferSetError(const virBufferPtr buf)
+virBufferSetError(virBufferPtr buf)
{
VIR_FREE(buf-content);
buf-size = 0;
@@ -113,7 +113,7 @@ virBufferAdd(const virBufferPtr buf, const char
*str, int len)
Maybe a typo here?The virBufferPtr still remains const, but the
declaration of this function in the buf.h shows this const is removed, too.
No. Rather, this is an artifact of git diff. When showing the context
line for a hunk, it uses the context from the pre-patch file (where the
'const' was still present), even though the hunk itself applies to the
post-patch version without 'const'. Basically, I never worry about the
@@ lines - they are there as hints for where to apply the patch, and not
a definitive part of the patch itself.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V1 4/6] Use scripting for cleaning and renaming of chains
Use scripts for the renaming and cleaning up of chains. This allows us to get
rid of some of the code that is only capable of renaming and removing chains
whose names are hardcoded.
A shell function 'collect_chains' is introduced that is given the name
of an ebtables chain and then recursively determines the names of all
chanins that are accessed from this chain and its sub-chains using 'jumps'.
This resulting list of chain names is then used to delete all the found
chains by first flushing and then deleting them.
The same function is also used for renaming temporary filters to their final
names.
I tested this with the bash and dash as script interpreters.
---
src/nwfilter/nwfilter_ebiptables_driver.c | 189 --
1 file changed, 102 insertions(+), 87 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -91,6 +91,37 @@ static char *gawk_cmd_path;
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
snprintf(buf, sizeof(buf), %c-%s-%s, prefix, ifname, suffix)
+#define FUNC_COLLECT_CHAINS \
+collect_chains()\n \
+{\n \
+ local sc\n \
+ sc=$(%s -t %s -L $1 | \\\n \
+ sed -n \/Bridge chain*/,$ s/.*\\-j \\([%s]-.*\\)/\\1/p\)\n \
+ for tmp in `echo \$sc\`; do\n \
+sc=\$sc $(collect_chains $tmp)\\n \
+ done\n \
+ echo \$sc\\n \
+}\n
+
+#define FUNC_DELETE_CHAINS \
+rm_chains()\n \
+{\n \
+ for tmp in `echo \$1\`; do %s -t %s -F $tmp; done\n \
+ for tmp in `echo \$1\`; do %s -t %s -X $tmp; done\n \
+}\n
+
+#define FUNC_RENAME_CHAINS \
+rename_chains()\n \
+{\n \
+ for tmp in `echo \$1\`; do\n \
+tmp2=`expr substr \$tmp\ 1 1`\n \
+if [ $tmp2 = \%c\ ]; then\n \
+%s -t %s -E \$tmp\ \%c\`expr substr \$tmp\ 2 33`\n \
+elif [ $tmp2 = \%c\ ]; then\n \
+%s -t %s -E \$tmp\ \%c\`expr substr \$tmp\ 2 33`\n \
+fi\n \
+ done\n \
+}\n
#define VIRT_IN_CHAIN libvirt-in
#define VIRT_OUT_CHAIN libvirt-out
@@ -2805,95 +2836,64 @@ ebtablesCreateTmpSubChain(virBufferPtr b
return 0;
}
-
-static int
-_ebtablesRemoveSubChain(virBufferPtr buf,
-int incoming,
-const char *ifname,
-enum l3_proto_idx protoidx,
-int isTempChain)
+static int _ebtablesRemoveSubChains(virBufferPtr buf,
+const char *ifname,
+const char *chains)
{
-char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
-char chainPrefix;
-
-if (isTempChain) {
-chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN_TEMP
-: CHAINPREFIX_HOST_OUT_TEMP;
-} else {
-chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN
-: CHAINPREFIX_HOST_OUT;
-}
+char rootchain[MAX_CHAINNAME_LENGTH];
+unsigned i;
-PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
-PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val);
-
-virBufferAsprintf(buf,
- %s -t %s -D %s -p 0x%x -j %s CMD_SEPARATOR
- %s -t %s -F %s CMD_SEPARATOR
- %s -t %s -X %s CMD_SEPARATOR,
+virBufferAsprintf(buf, FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
+virBufferAsprintf(buf, FUNC_DELETE_CHAINS,
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
- rootchain, l3_protocols[protoidx].attr, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE);
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+virBufferAddLit(buf, a=\);
+for (i = 0; chains[i] != 0; i++) {
+PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+virBufferAsprintf(buf, $(collect_chains %s) , rootchain);
+}
+virBufferAddLit(buf, \\n);
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+for (i = 0; chains[i] != 0; i++) {
+PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+virBufferAsprintf(buf,
+ %s -t %s -F %s\n,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
+ rootchain);
+}
+virBufferAddLit(buf, rm_chains \$a\\n);
return 0;
}
-
-static int
-ebtablesRemoveSubChain(virBufferPtr buf,
- int incoming,
- const char *ifname,
- enum l3_proto_idx protoidx)
-{
-return _ebtablesRemoveSubChain(buf,
- incoming, ifname, protoidx, 0);
-}
-
-
static int
ebtablesRemoveSubChains(virBufferPtr buf,
[libvirt] [PATCH V1 5/6] Use actual name of a chain in data structure
Use the name of the chain rather than its type index (enum).
This pushes the later enablement of chains with user-given names
into the XML parser. For now we still only allow those names that
are well known ('root', 'arp', 'rarp', 'ipv4' and 'ipv6').
---
src/conf/nwfilter_conf.c | 16
src/conf/nwfilter_conf.h |2 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 13 +++--
src/nwfilter/nwfilter_ebiptables_driver.h |2 +-
4 files changed, 21 insertions(+), 12 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -311,6 +311,7 @@ virNWFilterDefFree(virNWFilterDefPtr def
virNWFilterEntryFree(def-filterEntries[i]);
VIR_FREE(def-filterEntries);
+VIR_FREE(def-chainsuffix);
VIR_FREE(def);
}
@@ -2031,20 +2032,27 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
-ret-chainsuffix = VIR_NWFILTER_CHAINSUFFIX_ROOT;
chain = virXPathString(string(./@chain), ctxt);
if (chain) {
-if ((ret-chainsuffix =
- virNWFilterChainSuffixTypeFromString(chain)) 0) {
+if (virNWFilterChainSuffixTypeFromString(chain) 0) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
_(unknown chain suffix '%s'), chain);
goto cleanup;
}
+ret-chainsuffix = chain;
/* assign an implicit priority -- support XML attribute later */
if (intMapGetByString(chain_priorities, chain, 0,
ret-chainPriority) == false) {
ret-chainPriority = (MAX_RULE_PRIORITY + MIN_RULE_PRIORITY) / 2;
}
+chain = NULL;
+} else {
+ret-chainsuffix = strdup(virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT));
+if (ret-chainsuffix == NULL) {
+virReportOOMError();
+goto cleanup;
+}
}
uuid = virXPathString(string(./uuid), ctxt);
@@ -2910,7 +2918,7 @@ virNWFilterDefFormat(virNWFilterDefPtr d
virBufferAsprintf(buf, filter name='%s' chain='%s',
def-name,
- virNWFilterChainSuffixTypeToString(def-chainsuffix));
+ def-chainsuffix);
virBufferAddLit(buf, \n);
virUUIDFormat(def-uuid, uuid);
Index: libvirt-acl/src/conf/nwfilter_conf.h
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -444,7 +444,7 @@ struct _virNWFilterDef {
char *name;
unsigned char uuid[VIR_UUID_BUFLEN];
-int chainsuffix; /*enum virNWFilterChainSuffixType */
+char *chainsuffix;
virNWFilterChainPriority chainPriority;
int nentries;
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -357,7 +357,7 @@ ebiptablesRuleInstFree(ebiptablesRuleIns
static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
- enum virNWFilterChainSuffixType neededChain,
+ const char *neededChain,
virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
@@ -1933,11 +1933,13 @@ ebtablesCreateRuleInstance(char chainPre
goto err_exit;
}
-if (nwfilter-chainsuffix == VIR_NWFILTER_CHAINSUFFIX_ROOT)
+if (STREQ(nwfilter-chainsuffix,
+ virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT)))
PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
else
PRINT_CHAIN(chain, chainPrefix, ifname,
-virNWFilterChainSuffixTypeToString(nwfilter-chainsuffix));
+nwfilter-chainsuffix);
switch (rule-prtclType) {
@@ -2504,7 +2506,7 @@ ebiptablesDisplayRuleInstance(virConnect
ebiptablesRuleInstPtr inst = (ebiptablesRuleInstPtr)_inst;
VIR_INFO(Command Template: '%s', Needed protocol: '%s',
inst-commandTemplate,
- virNWFilterChainSuffixTypeToString(inst-neededProtocolChain));
+ inst-neededProtocolChain);
return 0;
}
@@ -3432,8 +3434,7 @@ ebiptablesApplyNewRules(virConnectPtr co
for (i = 0; i nruleInstances; i++) {
sa_assert (inst);
if (inst[i]-ruleType == RT_EBTABLES) {
-const char *name = virNWFilterChainSuffixTypeToString(
- inst[i]-neededProtocolChain);
+const char *name = inst[i]-neededProtocolChain;
if
[libvirt] [PATCH V1 1/6] Extend virHashTable with function to get hash tables keys
Add a function to the virHashTable for getting an array of the hash table's
keys and have the keys optionally sorted.
---
src/libvirt_private.syms |3 +
src/util/hash.c | 98 +++
src/util/hash.h | 14 ++
3 files changed, 115 insertions(+)
Index: libvirt-acl/src/util/hash.c
===
--- libvirt-acl.orig/src/util/hash.c
+++ libvirt-acl/src/util/hash.c
@@ -618,3 +618,101 @@ void *virHashSearch(virHashTablePtr tabl
return NULL;
}
+
+
+struct getKeysIter
+{
+virHashTablePtr table;
+void **array;
+virHashKeyValuePair *sortArray;
+unsigned arrayIdx;
+bool error;
+};
+
+static void virHashGetKeysIterator(void *payload,
+ const void *name, void *data)
+{
+struct getKeysIter *iter = data;
+void *key;
+
+if (iter-error)
+return;
+
+key = iter-table-keyCopy(name);
+
+if (key == NULL) {
+virReportOOMError();
+iter-error = true;
+return;
+}
+
+if (iter-sortArray) {
+iter-sortArray[iter-arrayIdx].key = key;
+iter-sortArray[iter-arrayIdx].value = payload;
+} else {
+iter-array[iter-arrayIdx] = iter-table-keyCopy(name);
+}
+iter-arrayIdx++;
+}
+
+void virHashFreeKeys(virHashTablePtr table, void **keys)
+{
+unsigned i = 0;
+
+if (keys == NULL)
+return;
+
+while (keys[i])
+table-keyFree(keys[i++]);
+
+VIR_FREE(keys);
+}
+
+typedef int (*qsort_comp)(const void *, const void *);
+
+void **virHashGetKeys(virHashTablePtr table,
+ virHashKeyComparator compar)
+{
+int i, numElems = virHashSize(table);
+struct getKeysIter iter = {
+.table = table,
+.arrayIdx = 0,
+.error = false,
+};
+
+if (numElems 0) {
+return NULL;
+}
+
+if (VIR_ALLOC_N(iter.array, numElems + 1)) {
+virReportOOMError();
+return NULL;
+}
+
+/* null-terminate array */
+iter.array[numElems] = NULL;
+
+if (compar
+VIR_ALLOC_N(iter.sortArray, numElems)) {
+VIR_FREE(iter.array);
+virReportOOMError();
+return NULL;
+}
+
+virHashForEach(table, virHashGetKeysIterator, iter);
+if (iter.error) {
+VIR_FREE(iter.sortArray);
+virHashFreeKeys(table, iter.array);
+return NULL;
+}
+
+if (compar) {
+qsort(iter.sortArray[0], numElems, sizeof(iter.sortArray[0]),
+ (qsort_comp)compar);
+for (i = 0; i numElems; i++)
+iter.array[i] = iter.sortArray[i].key;
+VIR_FREE(iter.sortArray);
+}
+
+return iter.array;
+}
Index: libvirt-acl/src/util/hash.h
===
--- libvirt-acl.orig/src/util/hash.h
+++ libvirt-acl/src/util/hash.h
@@ -130,6 +130,20 @@ void *virHashLookup(virHashTablePtr tabl
*/
void *virHashSteal(virHashTablePtr table, const void *name);
+/*
+ * Get the set of keys and have them optionally sorted.
+ */
+typedef struct _virHashKeyValuePair virHashKeyValuePair;
+typedef virHashKeyValuePair *virHashKeyValuePairPtr;
+struct _virHashKeyValuePair {
+void *key;
+const void *value;
+};
+typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr ,
+const virHashKeyValuePairPtr );
+void **virHashGetKeys(virHashTablePtr table, virHashKeyComparator compar);
+void virHashFreeKeys(virHashTablePtr table, void **);
+
/*
* Iterators
Index: libvirt-acl/src/libvirt_private.syms
===
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -552,6 +552,8 @@ virHashAddEntry;
virHashCreate;
virHashForEach;
virHashFree;
+virHashFreeKeys;
+virHashGetKeys;
virHashLookup;
virHashRemoveEntry;
virHashRemoveSet;
@@ -559,6 +561,7 @@ virHashSearch;
virHashSize;
virHashSteal;
virHashTableSize;
+virHashUpdateEntry;
# hooks.h
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V1 3/6] Make filter creation in root table more flexible
Use the previously introduced chain priorities to sort the chains for access
from the 'root' table and have them created in the proper order. This gets
rid of a lot of code that was previously creating the chains in a more
hardcoded way.
To detmerine what protocol a filter is used for evaluation do prefix-
matching, i.e., the filter 'arp' is used to filter for the 'arp' protocol,
'ipv4' for the 'ipv4' protocol and 'arp-xyz' will also be used to filter
for the 'arp' protocol.
---
src/nwfilter/nwfilter_ebiptables_driver.c | 130 ++
1 file changed, 98 insertions(+), 32 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2774,6 +2774,7 @@ ebtablesCreateTmpSubChain(virBufferPtr b
int incoming,
const char *ifname,
enum l3_proto_idx protoidx,
+ const char *filtername,
int stopOnError)
{
char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
@@ -2781,7 +2782,8 @@ ebtablesCreateTmpSubChain(virBufferPtr b
: CHAINPREFIX_HOST_OUT_TEMP;
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
-PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val);
+PRINT_CHAIN(chain, chainPrefix, ifname,
+(filtername) ? filtername : l3_protocols[protoidx].val);
virBufferAsprintf(buf,
CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR
@@ -3288,6 +3290,13 @@ ebiptablesRuleOrderSort(const void *a, c
return ((*insta)-priority - (*instb)-priority);
}
+static int
+ebiptablesFilterOrderSort(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+return *(virNWFilterChainPriority *)a-value -
+ *(virNWFilterChainPriority *)b-value;
+}
static void
iptablesCheckBridgeNFCallEnabled(bool isIPv6)
@@ -3327,6 +3336,56 @@ iptablesCheckBridgeNFCallEnabled(bool is
}
}
+/*
+ * Given a filtername determine the protocol it is used for evaluating
+ * We do prefix-matching to determine the protocol.
+ */
+static enum l3_proto_idx
+ebtablesGetProtoIdxByFiltername(const char *filtername)
+{
+enum l3_proto_idx idx;
+
+for (idx = 0; idx L3_PROTO_LAST_IDX; idx++) {
+if (STRPREFIX(filtername, l3_protocols[idx].val)) {
+return idx;
+}
+}
+
+return -1;
+}
+
+static int
+ebtablesCreateTmpRootAndSubChains(virBufferPtr buf,
+ const char *ifname,
+ virHashTablePtr chains, int direction)
+{
+int rc = 0, i;
+
+if (virHashSize(chains) != 0) {
+char **filter_names;
+
+ebtablesCreateTmpRootChain(buf, direction, ifname, 1);
+filter_names = (char **)virHashGetKeys(chains,
+ ebiptablesFilterOrderSort);
+if (filter_names == NULL) {
+virReportOOMError();
+rc = 1;
+goto err_exit;
+}
+for (i = 0; filter_names[i]; i++) {
+enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername(
+ filter_names[i]);
+if ((int)idx 0)
+continue;
+ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
+ filter_names[i], 1);
+}
+virHashFreeKeys(chains, (void **)filter_names);
+}
+
+ err_exit:
+return rc;
+}
static int
ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3337,24 +3396,43 @@ ebiptablesApplyNewRules(virConnectPtr co
int i;
int cli_status;
ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst;
-int chains_in = 0, chains_out = 0;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+virHashTablePtr chains_in_set = virHashCreate(10, NULL),
+chains_out_set = virHashCreate(10, NULL);
bool haveIptables = false;
bool haveIp6tables = false;
+if (!chains_in_set || !chains_out_set) {
+virReportOOMError();
+goto exit_free_sets;
+}
+
if (nruleInstances 1 inst)
qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort);
+/* scan the rules to see which chains need to be created */
for (i = 0; i nruleInstances; i++) {
sa_assert (inst);
if (inst[i]-ruleType == RT_EBTABLES) {
-if (inst[i]-chainprefix == CHAINPREFIX_HOST_IN_TEMP)
-chains_in |= (1 inst[i]-neededProtocolChain);
-else
-chains_out |= (1 inst[i]-neededProtocolChain);
+const char *name = virNWFilterChainSuffixTypeToString(
+
[libvirt] [PATCH V1 0/6] Make part of inner workings of nwfilters more flexible
The following series of patches re-does some of the inner workings
of nwfilters with the goal to enable users to write filters that have other
than the system-known chains supported right now ('root','arp','rarp','ipv4'
and 'ipv6'). Ideally users should be able to provide a chain name in the
chains XML attribute and either be able to jump to it as an 'action' or
have the chain created automatically as it is the case right now for those
chains enumerated before.
I am introducing internal priorities for the chains mentioned above so that
their creation can be made more flexible -- currently their creation and
the order in which they are accessed is hardcoded. This largely does away
with the hardcoded stuff.
Further, filters will be automatically accessed from the (ebtables)
'root' chain using the prefix of the name of the chain. As an example, this
filter will be accessed from the root chain for 'arp' packets since its
name 'arpmac' has the prefix 'arp'.
filter name='allow-arpmac' chain='arpmac'
uuid94abeecc-c956-0ac8-1f49-a06ee8995688/uuid
rule action='accept' direction='out' priority='100'
arp opcode='Request_Reverse' arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC'
arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/
/rule
rule action='accept' direction='inout' priority='500'/
/filter
In the future the chains may have priorities supported in the XML in order to
control the order in which they are accessed.
filter name='allow-arpmac' chain='arpmac' prirotiy='650'
[...]
The series does not enable users yet to provide names of chains but instead
pushes the problem of their later enablement into the XML parser and
prepares the rest of the code to handle it (as far as this can be seen).
I did the testing with the test cases in libvirt-tck and did not see
regressions.
Regards,
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V1 6/6] Example of adding a new filter called arpmac.
Only as an example show how to add a new 'system filter' called 'arpmac'.
Using 'prefix matching' introduced in previous patches, it generates
a table 'arpmac' that will be jumped into using '-p arp'.
The below patch adds arpmac with a priority of 650, which helps sorting
its entry in the 'root' table.
Since previous code still doesn't allow arbitrary names we still need to
add its name to the virNWFilerChainSuffixType and the list of strings.
This patch would enable the following filter using the 'arpmac' chain.
filter name='allow-arpmac' chain='arpmac'
uuid94abeecc-c956-0ac8-1f49-a06ee8995688/uuid
rule action='accept' direction='out' priority='100'
arp opcode='Request_Reverse' arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC'
arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/
/rule
rule action='accept' direction='inout' priority='500'/
/filter
---
src/conf/nwfilter_conf.c |5 -
src/conf/nwfilter_conf.h |1 +
2 files changed, 5 insertions(+), 1 deletion(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -81,7 +81,8 @@ VIR_ENUM_IMPL(virNWFilterChainSuffix, VI
arp,
rarp,
ipv4,
- ipv6);
+ ipv6,
+ arpmac);
VIR_ENUM_IMPL(virNWFilterRuleProtocol, VIR_NWFILTER_RULE_PROTOCOL_LAST,
none,
@@ -124,6 +125,7 @@ struct int_map {
#define NWFILTER_IPV4_FILTER_PRI 200
#define NWFILTER_IPV6_FILTER_PRI 400
#define NWFILTER_ARP_FILTER_PRI 600
+#define NWFILTER_ARPMAC_FILTER_PRI 650
#define NWFILTER_RARP_FILTER_PRI 800
static const struct int_map chain_priorities[] = {
@@ -132,6 +134,7 @@ static const struct int_map chain_priori
INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, ipv6),
INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , arp ),
INTMAP_ENTRY(NWFILTER_RARP_FILTER_PRI, rarp),
+INTMAP_ENTRY(NWFILTER_ARPMAC_FILTER_PRI, arpmac),
INTMAP_ENTRY_LAST,
};
Index: libvirt-acl/src/conf/nwfilter_conf.h
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -431,6 +431,7 @@ enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_RARP,
VIR_NWFILTER_CHAINSUFFIX_IPv4,
VIR_NWFILTER_CHAINSUFFIX_IPv6,
+VIR_NWFILTER_CHAINSUFFIX_ARPMAC,
VIR_NWFILTER_CHAINSUFFIX_LAST,
};
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix two comments related to error handling
Signed-off-by: Philipp Hahn h...@univention.de
---
include/libvirt/virterror.h |2 +-
python/libvirt-override.py |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index dfbe2bc..a8549b7 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -206,7 +206,7 @@ typedef enum {
VIR_ERR_INVALID_STORAGE_VOL = 47, /* invalid storage vol object */
VIR_WAR_NO_STORAGE = 48, /* failed to start storage */
VIR_ERR_NO_STORAGE_POOL = 49, /* storage pool not found */
-VIR_ERR_NO_STORAGE_VOL = 50, /* storage pool not found */
+VIR_ERR_NO_STORAGE_VOL = 50, /* storage volume not found */
VIR_WAR_NO_NODE = 51, /* failed to start node driver */
VIR_ERR_INVALID_NODE_DEVICE = 52, /* invalid node device object */
VIR_ERR_NO_NODE_DEVICE = 53, /* node device not found */
diff --git a/python/libvirt-override.py b/python/libvirt-override.py
index 387fddf..8427eab 100644
--- a/python/libvirt-override.py
+++ b/python/libvirt-override.py
@@ -79,7 +79,7 @@ class libvirtError(Exception):
# register the libvirt global error handler
#
def registerErrorHandler(f, ctx):
-Register a Python written function to for error reporting.
+Register a Python function for error reporting.
The function is called back as f(ctx, error), with error
being a list of information about the error being raised.
Returns 1 in case of success.
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix two comments related to error handling
On 10/17/2011 05:02 PM, Philipp Hahn wrote: Signed-off-by: Philipp Hahnh...@univention.de --- include/libvirt/virterror.h |2 +- python/libvirt-override.py |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) ACK and pushed. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Compiler warnings
Hi all, I meet the following compiler warnings today: .. CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] CC libvirt_util_la-command.lo .. CC libvirt_lxc-buf.o util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] CC libvirt_lxc-command.o .. Has anybody the same experience? Thanks, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Compiler warnings
On 10/17/2011 09:35 AM, Alex Jia wrote: Hi all, I meet the following compiler warnings today: .. CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Which version of gcc? Has anybody the same experience? Not with gcc 4.5.1 (Fedora 14), but not the first time that newer gcc has become smarter. I'll patch it if I can reproduce it (I'll try a rawhide build shortly). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 03/10] reverse sense of address matching
On 10/12/2011 03:50 PM, David L Stevens wrote: This patch changes rules of the form: if ! addr drop accept to: if addr return ... drop The patch adds a mac chain to do a mac address list and separates the arp chain into separate arpmac and arpip chains that can check multiple MAC or IP addresses in any combination. This patch itself does not support multiple addresses via the MAC and IP variables, but only changes the form of the rules to allow multiple addresses in the future. Signed-off-by: David L Stevensdlstev...@us.ibm.com --- examples/xml/nwfilter/Makefile.am |4 ++ examples/xml/nwfilter/allow-arp.xml |5 ++- examples/xml/nwfilter/allow-arpip.xml |3 ++ examples/xml/nwfilter/allow-arpmac.xml|3 ++ examples/xml/nwfilter/clean-traffic.xml |6 ++-- examples/xml/nwfilter/no-arp-spoofing.xml | 19 ++ examples/xml/nwfilter/no-arpip-spoofing.xml | 12 ++ examples/xml/nwfilter/no-arpmac-spoofing.xml |7 examples/xml/nwfilter/no-ip-spoofing.xml |6 ++- examples/xml/nwfilter/no-mac-spoofing.xml | 12 -- examples/xml/nwfilter/no-other-l2-traffic.xml | 13 +-- src/conf/nwfilter_conf.c |4 ++- src/conf/nwfilter_conf.h |4 ++- src/nwfilter/nwfilter_ebiptables_driver.c | 48 +--- 14 files changed, 99 insertions(+), 47 deletions(-) create mode 100644 examples/xml/nwfilter/allow-arpip.xml create mode 100644 examples/xml/nwfilter/allow-arpmac.xml create mode 100644 examples/xml/nwfilter/no-arpip-spoofing.xml create mode 100644 examples/xml/nwfilter/no-arpmac-spoofing.xml diff --git a/examples/xml/nwfilter/Makefile.am b/examples/xml/nwfilter/Makefile.am index 23fd753..84aaa3c 100644 --- a/examples/xml/nwfilter/Makefile.am +++ b/examples/xml/nwfilter/Makefile.am @@ -3,12 +3,16 @@ FILTERS = \ allow-arp.xml \ + allow-arpip.xml \ + allow-arpmac.xml \ allow-dhcp-server.xml \ allow-dhcp.xml \ allow-incoming-ipv4.xml \ allow-ipv4.xml \ clean-traffic.xml \ no-arp-spoofing.xml \ + no-arpmac-spoofing.xml \ + no-arpip-spoofing.xml \ no-ip-multicast.xml \ no-ip-spoofing.xml \ no-mac-broadcast.xml \ diff --git a/examples/xml/nwfilter/allow-arp.xml b/examples/xml/nwfilter/allow-arp.xml index 63a92b2..6271ae4 100644 --- a/examples/xml/nwfilter/allow-arp.xml +++ b/examples/xml/nwfilter/allow-arp.xml @@ -1,3 +1,4 @@ -filter name='allow-arp' chain='arp' -rule direction='inout' action='accept'/ +filter name='allow-arp' chain='arpmac' +filterref filter='allow-arpmac.xml'/ +filterref filter='allow-arpip.xml'/ /filter diff --git a/examples/xml/nwfilter/allow-arpip.xml b/examples/xml/nwfilter/allow-arpip.xml new file mode 100644 index 000..6ddb6fe --- /dev/null +++ b/examples/xml/nwfilter/allow-arpip.xml @@ -0,0 +1,3 @@ +filter name='allow-arpip' chain='arpip' +rule direction='inout' action='accept'/ +/filter diff --git a/examples/xml/nwfilter/allow-arpmac.xml b/examples/xml/nwfilter/allow-arpmac.xml new file mode 100644 index 000..54f6714 --- /dev/null +++ b/examples/xml/nwfilter/allow-arpmac.xml @@ -0,0 +1,3 @@ +filter name='allow-arpmac' chain='arpmac' +rule direction='inout' action='accept'/ +/filter diff --git a/examples/xml/nwfilter/clean-traffic.xml b/examples/xml/nwfilter/clean-traffic.xml index 40f0ecb..9cee799 100644 --- a/examples/xml/nwfilter/clean-traffic.xml +++ b/examples/xml/nwfilter/clean-traffic.xml @@ -11,10 +11,10 @@ !-- preventing ARP spoofing/poisoning -- filterref filter='no-arp-spoofing'/ -!-- preventing any other traffic than IPv4 and ARP -- -filterref filter='no-other-l2-traffic'/ - !-- allow qemu to send a self-announce upon migration end -- filterref filter='qemu-announce-self'/ +!-- preventing any other traffic than IPv4 and ARP -- +filterref filter='no-other-l2-traffic'/ + /filter diff --git a/examples/xml/nwfilter/no-arp-spoofing.xml b/examples/xml/nwfilter/no-arp-spoofing.xml index 3c83acd..1979b20 100644 --- a/examples/xml/nwfilter/no-arp-spoofing.xml +++ b/examples/xml/nwfilter/no-arp-spoofing.xml @@ -1,17 +1,4 @@ -filter name='no-arp-spoofing' chain='arp' -uuidf88f1932-debf-4aa1-9fbe-f10d3aa4bc95/uuid -rule action='drop' direction='out' priority='300' -mac match='no' srcmacaddr='$MAC'/ -/rule - -!-- no arp spoofing -- -!-- drop if ipaddr or macaddr does not belong to guest -- -rule action='drop' direction='out' priority='350' -arp match='no' arpsrcmacaddr='$MAC'/ -/rule -rule action='drop' direction='out' priority='400' -arp match='no' arpsrcipaddr='$IP' / -/rule -!-- allow everything else -- -rule action='accept' direction='in' priority='425' / +filter name='no-arp-spoofing' +filterref filter='no-arpmac-spoofing' / +filterref filter='no-arpip-spoofing' / /filter diff --git
Re: [libvirt] Compiler warnings
Eric, # rpm -q gcc gcc-4.4.6-3.el6.x86_64 Regards, Alex - Original Message - From: Eric Blake ebl...@redhat.com To: Alex Jia a...@redhat.com Cc: libvir-list@redhat.com Sent: Monday, October 17, 2011 11:38:50 PM Subject: Re: [libvirt] Compiler warnings On 10/17/2011 09:35 AM, Alex Jia wrote: Hi all, I meet the following compiler warnings today: .. CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Which version of gcc? Has anybody the same experience? Not with gcc 4.5.1 (Fedora 14), but not the first time that newer gcc has become smarter. I'll patch it if I can reproduce it (I'll try a rawhide build shortly). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy DROP
On 10/12/2011 03:50 PM, David L Stevens wrote: This patch simplifies the table rules by setting the protocol chains policy to be DROP and removes the explicit -j DROP entries that the protocol rules had previously. It also makes no-other-rarp-traffic.xml obsolete. I agree with Daniel's previous comments that this could introduce compatibility problems. It would be best not to change it or if really need be later on introduce an XML attribute for a chain that allows to choose whether the default policy is accept or drop. Stefan Signed-off-by: David L Stevensdlstev...@us.ibm.com --- examples/xml/nwfilter/Makefile.am |1 - examples/xml/nwfilter/no-arpip-spoofing.xml |2 -- examples/xml/nwfilter/no-arpmac-spoofing.xml|2 -- examples/xml/nwfilter/no-ip-spoofing.xml|2 -- examples/xml/nwfilter/no-mac-spoofing.xml |2 -- examples/xml/nwfilter/no-other-rarp-traffic.xml |3 --- examples/xml/nwfilter/qemu-announce-self.xml|1 - src/nwfilter/nwfilter_ebiptables_driver.c | 11 +-- 8 files changed, 1 insertions(+), 23 deletions(-) delete mode 100644 examples/xml/nwfilter/no-other-rarp-traffic.xml diff --git a/examples/xml/nwfilter/Makefile.am b/examples/xml/nwfilter/Makefile.am index 84aaa3c..67085fa 100644 --- a/examples/xml/nwfilter/Makefile.am +++ b/examples/xml/nwfilter/Makefile.am @@ -18,7 +18,6 @@ FILTERS = \ no-mac-broadcast.xml \ no-mac-spoofing.xml \ no-other-l2-traffic.xml \ - no-other-rarp-traffic.xml \ qemu-announce-self.xml \ qemu-announce-self-rarp.xml diff --git a/examples/xml/nwfilter/no-arpip-spoofing.xml b/examples/xml/nwfilter/no-arpip-spoofing.xml index ee42d40..7ef6f0f 100644 --- a/examples/xml/nwfilter/no-arpip-spoofing.xml +++ b/examples/xml/nwfilter/no-arpip-spoofing.xml @@ -7,6 +7,4 @@ rule action='return' direction='out' priority='410' arp match='yes' arpsrcipaddr='0.0.0.0' / /rule -!-- drop everything else -- -rule action='drop' direction='out' priority='1000' / /filter diff --git a/examples/xml/nwfilter/no-arpmac-spoofing.xml b/examples/xml/nwfilter/no-arpmac-spoofing.xml index 90499d3..3834047 100644 --- a/examples/xml/nwfilter/no-arpmac-spoofing.xml +++ b/examples/xml/nwfilter/no-arpmac-spoofing.xml @@ -2,6 +2,4 @@ rule action='return' direction='out' priority='350' arp match='yes' arpsrcmacaddr='$MAC'/ /rule -!-- drop everything else -- -rule action='drop' direction='out' priority='1000' / /filter diff --git a/examples/xml/nwfilter/no-ip-spoofing.xml b/examples/xml/nwfilter/no-ip-spoofing.xml index 84e8a5e..2fccd12 100644 --- a/examples/xml/nwfilter/no-ip-spoofing.xml +++ b/examples/xml/nwfilter/no-ip-spoofing.xml @@ -4,6 +4,4 @@ rule action='return' direction='out' ip match='yes' srcipaddr='$IP' / /rule -!-- drop any that don't match the source IP list -- -rule action='drop' direction='out' / /filter diff --git a/examples/xml/nwfilter/no-mac-spoofing.xml b/examples/xml/nwfilter/no-mac-spoofing.xml index aee56c7..e2e8c03 100644 --- a/examples/xml/nwfilter/no-mac-spoofing.xml +++ b/examples/xml/nwfilter/no-mac-spoofing.xml @@ -4,6 +4,4 @@ rule action='return' direction='out' priority='350' mac match='yes' srcmacaddr='$MAC'/ /rule -!-- drop everything else -- -rule action='drop' direction='out' priority='1000' / /filter diff --git a/examples/xml/nwfilter/no-other-rarp-traffic.xml b/examples/xml/nwfilter/no-other-rarp-traffic.xml deleted file mode 100644 index 7729996..000 --- a/examples/xml/nwfilter/no-other-rarp-traffic.xml +++ /dev/null @@ -1,3 +0,0 @@ -filter name='no-other-rarp-traffic' chain='rarp' -rule action='drop' direction='inout' priority='1000'/ -/filter diff --git a/examples/xml/nwfilter/qemu-announce-self.xml b/examples/xml/nwfilter/qemu-announce-self.xml index 352db50..12957b5 100644 --- a/examples/xml/nwfilter/qemu-announce-self.xml +++ b/examples/xml/nwfilter/qemu-announce-self.xml @@ -8,6 +8,5 @@ !-- accept if it was changed to rarp -- filterref filter='qemu-announce-self-rarp'/ -filterref filter='no-other-rarp-traffic'/ /filter diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 3c6fca7..e6a4880 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2791,7 +2791,7 @@ ebtablesCreateTmpSubChain(virBufferPtr buf, protostr[0] = '\0'; virBufferAsprintf(buf, - CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR + CMD_DEF(%s -t %s -N %s -P DROP) CMD_SEPARATOR CMD_EXEC %s CMD_DEF(%s -t %s -A %s %s -j %s) CMD_SEPARATOR @@ -3015,15 +3015,6 @@ ebtablesApplyBasicRules(const char *ifname, PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); virBufferAsprintf(buf, -
Re: [libvirt] [PATCH] Fix VPATH build
On 10/17/2011 09:54 AM, Jiri Denemark wrote: probes.h is generated in build directory; setting a dependency on probes.h from source directory doesn't work well in VPATH builds. Caused by commit 1afcfbdda0cac112faa61f74ec943e46aa43f2f5 --- src/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 510e5ef..87d91ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -191,7 +191,7 @@ REMOTE_DRIVER_GENERATED = \ # The remote RPC driver needs probes.h if WITH_DTRACE -REMOTE_DRIVER_GENERATED += $(srcdir)/probes.h +REMOTE_DRIVER_GENERATED += probes.h ACK. probes.h is part of BUILT_SOURCES, which means it is not part of the tarball, which means that it does not have to be in the source directory. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: avoid dereferencing a NULL pointer
From: Alex Jia a...@redhat.com * src/qemu/qemu_hostdev.c: function 'pciDeviceListFind' probably explicitly returns null, however, the function 'pciDeviceSetUsedBy' directly uses it without any judgement. Signed-off-by: Alex Jia a...@redhat.com --- src/qemu/qemu_hostdev.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index c65f6f5..4e148b0 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -227,9 +227,8 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, pciDevice *dev, *activeDev; dev = pciDeviceListGet(pcidevs, i); -activeDev = pciDeviceListFind(driver-activePciHostdevs, dev); - -pciDeviceSetUsedBy(activeDev, name); +if ((activeDev = pciDeviceListFind(driver-activePciHostdevs, dev))) +pciDeviceSetUsedBy(activeDev, name); } /* Loop 6: Now steal all the devices from pcidevs */ -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Compiler warnings
On 10/17/2011 09:51 AM, Alex Jia wrote: Eric, # rpm -q gcc gcc-4.4.6-3.el6.x86_64 I'm still not seeing it on my RHEL 6 host. What version of glibc (in case it's a header problem), and is this the latest libvirt.git or a particular source rpm rebuild? What's on line 430 of the file in question? CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification
On 10/12/2011 03:50 PM, David L Stevens wrote:
This patch adds the internal capability to add rules to existing
chains instead of using temporary chains and to generate placeholders for
chains that are referenced without generating a rule for them immediately.
Finally, it includes variable matching for filter instantiation
(i.e., instantiate only when a given variable is present in a filter, or
only when it is not).
Following the above I am not sure what this will be used for as part of
this extension.
Signed-off-by: David L Stevensdlstev...@us.ibm.com
---
src/conf/nwfilter_conf.h |4 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 93 +
src/nwfilter/nwfilter_gentech_driver.c| 32 +-
3 files changed, 100 insertions(+), 29 deletions(-)
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index 17e954e..4348378 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -525,7 +525,9 @@ typedef int (*virNWFilterRuleCreateInstance)(virConnectPtr
conn,
virNWFilterRuleDefPtr rule,
const char *ifname,
virNWFilterHashTablePtr vars,
- virNWFilterRuleInstPtr res);
+ virNWFilterRuleInstPtr res,
+ bool usetemp,
+ bool dummy);
typedef int (*virNWFilterRuleApplyNewRules)(virConnectPtr conn,
const char *ifname,
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index e6a4880..918625c 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -1136,6 +1136,7 @@ iptablesEnforceDirection(int directionIn,
* @isIPv6 : Whether this is an IPv6 rule
* @maySkipICMP : whether this rule may under certain circumstances skip
* the ICMP rule from being created
+ * @dummy : generate rule placeholder without installing
*
* Convert a single rule into its representation for later instantiation
*
@@ -1154,7 +1155,8 @@ _iptablesCreateRuleInstance(int directionIn,
const char *match, bool defMatch,
const char *accept_target,
bool isIPv6,
-bool maySkipICMP)
+bool maySkipICMP,
+bool dummy)
{
char chain[MAX_CHAINNAME_LENGTH];
char number[20];
@@ -1181,6 +1183,13 @@ _iptablesCreateRuleInstance(int directionIn,
PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname);
+if (dummy) {
+virBufferAsprintf(buf, CMD_DEF_PRE %s -- %s -%%c %s %%s,
+ echo, iptables_cmd, chain);
+bufUsed = virBufferUse(buf);
+goto prskip;
+}
+
switch (rule-prtclType) {
case VIR_NWFILTER_RULE_PROTOCOL_TCP:
case VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6:
@@ -1521,6 +1530,8 @@ _iptablesCreateRuleInstance(int directionIn,
return -1;
}
+prskip:
+
if ((srcMacSkipped bufUsed == virBufferUse(buf)) ||
skipRule) {
virBufferFreeAndReset(buf);
@@ -1636,7 +1647,9 @@ iptablesCreateRuleInstanceStateCtrl(virNWFilterDefPtr
nwfilter,
const char *ifname,
virNWFilterHashTablePtr vars,
virNWFilterRuleInstPtr res,
-bool isIPv6)
+bool isIPv6,
+bool usetemp,
+bool dummy)
{
int rc;
int directionIn = 0;
@@ -1668,7 +1681,7 @@ iptablesCreateRuleInstanceStateCtrl(virNWFilterDefPtr
nwfilter,
return 1;
}
-chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP;
+chainPrefix[1] = usetemp ? CHAINPREFIX_HOST_IN_TEMP : CHAINPREFIX_HOST_IN;
if (create) {
rc = _iptablesCreateRuleInstance(directionIn,
chainPrefix,
@@ -1680,7 +1693,8 @@ iptablesCreateRuleInstanceStateCtrl(virNWFilterDefPtr
nwfilter,
matchState, false,
RETURN,
isIPv6,
- maySkipICMP);
+ maySkipICMP,
+ dummy);
VIR_FREE(matchState);
if (rc)
@@ -1700,7 +1714,8 @@ iptablesCreateRuleInstanceStateCtrl(virNWFilterDefPtr
nwfilter,
return 1;
}
-chainPrefix[1] = CHAINPREFIX_HOST_OUT_TEMP;
+chainPrefix[1] = usetemp ?
Re: [libvirt] Compiler warnings
Eric,
It's latest libvirt upstream, current commit id is commit 0a71c79.
# rpm -q glibc
glibc-2.12-1.42.el6.x86_64
399 void
400 virBufferEscape(const virBufferPtr buf,
401 const char *toescape,
402 const char *format,
403 const char *str)
404 {
.
427 cur = str;
428 out = escaped;
429 while (*cur != 0) {
430 if (strchr(toescape, *cur))
431 *out++ = '\\';
432 *out++ = *cur;
433 cur++;
434 }
..
Regards,
Alex
- Original Message -
From: Eric Blake ebl...@redhat.com
To: Alex Jia a...@redhat.com
Cc: libvir-list@redhat.com
Sent: Tuesday, October 18, 2011 12:11:19 AM
Subject: Re: [libvirt] Compiler warnings
On 10/17/2011 09:51 AM, Alex Jia wrote:
Eric,
# rpm -q gcc
gcc-4.4.6-3.el6.x86_64
I'm still not seeing it on my RHEL 6 host. What version of glibc (in
case it's a header problem), and is this the latest libvirt.git or a
particular source rpm rebuild? What's on line 430 of the file in question?
CC libvirt_util_la-buf.lo
util/buf.c: In function 'virBufferEscape':
util/buf.c:430: warning: logical '' with non-zero constant will always
evaluate as true [-Wlogical-op]
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: avoid dereferencing a NULL pointer
On 10/17/2011 10:09 AM, a...@redhat.com wrote: From: Alex Jiaa...@redhat.com * src/qemu/qemu_hostdev.c: function 'pciDeviceListFind' probably explicitly returns null, however, the function 'pciDeviceSetUsedBy' directly uses it without any judgement. Signed-off-by: Alex Jiaa...@redhat.com --- src/qemu/qemu_hostdev.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index c65f6f5..4e148b0 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -227,9 +227,8 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, pciDevice *dev, *activeDev; dev = pciDeviceListGet(pcidevs, i); -activeDev = pciDeviceListFind(driver-activePciHostdevs, dev); - -pciDeviceSetUsedBy(activeDev, name); +if ((activeDev = pciDeviceListFind(driver-activePciHostdevs, dev))) +pciDeviceSetUsedBy(activeDev, name); False positive. Just a few lines earlier, in loop 4, we guaranteed that dev was added to driver-activePciHostdevs, therefore, activeDev cannot be NULL here. That said, we could probably simplify things by consolidating loop 5 and 6 into one, and in the process of that simplification, silence the spurious warning from the static analyzer. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix virt-sanlock-cleanup documentation
The referenced page does not exist, but locking.html has a section about sanlock. Signed-off-by: Philipp Hahn h...@univention.de --- tools/virt-sanlock-cleanup.in |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virt-sanlock-cleanup.in b/tools/virt-sanlock-cleanup.in index 72cd5e0..e143e7d 100644 --- a/tools/virt-sanlock-cleanup.in +++ b/tools/virt-sanlock-cleanup.in @@ -64,8 +64,8 @@ active. =head1 EXIT STATUS -Upon successful processing of leases cleanup, the exit status -will be 0 will be set. Upon fatal error a non-zero status will +Upon successful processing of leases cleanup, an exit status +of 0 will be set. Upon fatal error a non-zero status will be set. =head1 AUTHOR @@ -91,6 +91,6 @@ PURPOSE =head1 SEE ALSO -Cvirsh(1), online instructions Chttp://libvirt.org/locksanlock.html +Cvirsh(1), online instructions Chttp://libvirt.org/locking.html =cut -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 07/10] support variable value changing
On 10/12/2011 03:50 PM, David L Stevens wrote:
This patch adds a function that applies or deletes filter rules to existing
chains. Rules referencing the given variable are instantiated with the given
value, or optionally deleted. For example, passing variable IP with different
values will install rules using the IP variable with each of the different
values. These rules can later be removed by calling this function with the
same variable and value and delete argument set to 1.
Signed-off-by: David L Stevensdlstev...@us.ibm.com
---
src/nwfilter/nwfilter_gentech_driver.c | 86
src/nwfilter/nwfilter_gentech_driver.h | 11
2 files changed, 97 insertions(+), 0 deletions(-)
diff --git a/src/nwfilter/nwfilter_gentech_driver.c
b/src/nwfilter/nwfilter_gentech_driver.c
index 79350ac..563a1f3 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -620,6 +620,92 @@ virNWFilterRuleInstancesToArray(int nEntries,
/**
+ * virNWFilterChangeVar:
+ * @conn: pointer to virConnect object
+ * @techdriver: The driver to use for instantiation
+ * @filter: The filter to instantiate
+ * @ifname: The name of the interface to apply the rules to
+ * @vars: A map holding variable names and values used for instantiating
+ * the filter and its subfilters.
+ * @var: name of variable to change
+ * @value: value of variable to change
+ * @delete: =0 to create or =1 to delete the rules
+ *
+ * Returns 0 on success, a value otherwise.
+ *
+ * Instantiate or delete a filter and all subfilters with variable var
+ * set to value value.
+ * The name of the interface to which the rules belong must be
+ * provided.
+ *
+ * Call this function while holding the NWFilter filter update lock
+ */
+int
+virNWFilterChangeVar(virConnectPtr conn,
+virNWFilterTechDriverPtr techdriver,
+enum virDomainNetType nettype,
+virNWFilterDefPtr filter,
+const char *ifname,
+virNWFilterHashTablePtr vars,
+virNWFilterDriverStatePtr driver,
+const char *var,
+char *value,
+bool delete)
+{
+int rc;
+int j, nptrs;
+int nEntries = 0;
+virNWFilterRuleInstPtr *insts = NULL;
+void **ptrs = NULL;
+bool foundNewFilter = 0;
+
+if (virNWFilterHashTablePut(vars, var, value, 1)) {
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _(Cound not add
+ variable \%s\ to hashmap), var);
+return 1;
+}
+rc = _virNWFilterInstantiateRec(conn,
+techdriver,
+nettype,
+filter,
+ifname,
+vars,
+NWFILTER_STD_VAR_IP, 0,
+nEntries,insts,
+INSTANTIATE_ALWAYS,foundNewFilter,
+driver);
Given the NWFILTER_STD_VAR_IP parameter, what does it give us at this point?
+ if (rc)
+ goto err_exit;
+ rc = virNWFilterRuleInstancesToArray(nEntries, insts,ptrs,nptrs);
+ if (rc)
+ goto err_exit;
+
+if (virNWFilterHashTableRemoveEntry(vars, var) 0) {
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _(Could not remove
+ variable \%s\ from hashmap), var);
+return 1;
+}
+
+if (virNWFilterLockIface(ifname))
+ goto err_exit;
+
+ if (delete)
+ rc = techdriver-removeRules(conn, ifname, nptrs, ptrs);
+ else
+ rc = techdriver-addRules(conn, ifname, nptrs, ptrs);
I am wondering about this addRules() and whether the rules are being
added to the end of a chain and thus the rules' assumed priority would
have to be such that these rules can actually always be the last ones?
+ virNWFilterUnlockIface(ifname);
+ VIR_FREE(ptrs);
+
+err_exit:
+
+for (j = 0; j nEntries; j++)
+ virNWFilterRuleInstFree(insts[j]);
+VIR_FREE(insts);
+return rc;
+}
+
+
+/**
* virNWFilterInstantiate:
* @conn: pointer to virConnect object
* @techdriver: The driver to use for instantiation
diff --git a/src/nwfilter/nwfilter_gentech_driver.h
b/src/nwfilter/nwfilter_gentech_driver.h
index fa86030..34e95c7 100644
--- a/src/nwfilter/nwfilter_gentech_driver.h
+++ b/src/nwfilter/nwfilter_gentech_driver.h
@@ -48,6 +48,17 @@ int virNWFilterRollbackUpdateFilter(virConnectPtr conn,
int virNWFilterTearOldFilter(virConnectPtr conn,
const virDomainNetDefPtr net);
+int virNWFilterChangeVar(virConnectPtr conn,
+virNWFilterTechDriverPtr techdriver,
+enum virDomainNetType nettype,
+virNWFilterDefPtr filter,
+const char *ifname,
+
Re: [libvirt] [PATCH] qemu: avoid dereferencing a NULL pointer
- Original Message - From: Eric Blake ebl...@redhat.com False positive. Just a few lines earlier, in loop 4, we guaranteed that dev was added to driver-activePciHostdevs, therefore, activeDev cannot be NULL here. That said, we could probably simplify things by consolidating loop 5 and 6 into one, and in the process of that simplification, silence the it should consolidate loop 4 and loop 5 into one :-)however, loop 6 steps are a clear design if don't consider Coverity complains. Alex spurious warning from the static analyzer. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 09/10] add leasefile support
On 10/12/2011 03:50 PM, David L Stevens wrote:
This patch adds support for saving DHCP snooping leases to an on-disk
file and restoring saved leases that are still active on restart.
Signed-off-by: David L Stevensdlstev...@us.ibm.com
---
src/nwfilter/nwfilter_dhcpsnoop.c | 370 +++--
1 files changed, 353 insertions(+), 17 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c
b/src/nwfilter/nwfilter_dhcpsnoop.c
index f784a29..eedf550 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -56,10 +56,21 @@
#include nwfilter_gentech_driver.h
#include nwfilter_ebiptables_driver.h
#include nwfilter_dhcpsnoop.h
+#include configmake.h
#define VIR_FROM_THIS VIR_FROM_NWFILTER
+#define LEASEFILE LOCALSTATEDIR /run/libvirt/network/nwfilter.leases
+#define TMPLEASEFILE LOCALSTATEDIR /run/libvirt/network/nwfilter.ltmp
+static int lease_fd = -1;
+static int nleases = 0; /* number of active leases */
+static int wleases = 0; /* number of written leases */
+
static virHashTablePtr SnoopReqs;
+static pthread_mutex_t SnoopLock;
+
+#define snoop_lock(){ pthread_mutex_lock(SnoopLock); }
+#define snoop_unlock() { pthread_mutex_unlock(SnoopLock); }
struct virNWFilterSnoopReq {
virConnectPtr conn;
@@ -90,7 +101,14 @@ struct iplease {
static struct iplease *ipl_getbyip(struct iplease *start, uint32_t ipaddr);
static void ipl_update(struct iplease *pl, uint32_t timeout);
-
+
+static struct iflease *getiflease(const char *ifname);
+static void lease_open(void);
+static void lease_close(void);
+static void lease_load(void);
+static void lease_save(struct iplease *ipl);
+static void lease_refresh(void);
+static void lease_restore(struct virNWFilterSnoopReq *req);
/*
* ipl_ladd - add an IP lease to a list
@@ -150,6 +168,7 @@ ipl_add(struct iplease *plnew)
ipl_update(pl, plnew-ipl_timeout);
return;
}
+nleases++;
if (VIR_ALLOC(pl) 0) {
virReportOOMError();
return;
@@ -184,6 +203,7 @@ ipl_add(struct iplease *plnew)
return;
}
ipl_tadd(pl);
+lease_save(pl);
}
/*
@@ -231,6 +251,7 @@ ipl_del(struct iplease *ipl)
req = ipl-ipl_req;
ipl_tdel(ipl);
+lease_save(ipl);
if (inet_ntop(AF_INET,ipl-ipl_ipaddr, ipbuf, sizeof(ipbuf))) {
ipstr = strdup(ipbuf);
@@ -248,6 +269,7 @@ ipl_del(struct iplease *ipl)
_(ipl_del inet_ntop failed (0x%08X)),
ipl-ipl_ipaddr);
VIR_FREE(ipl);
+nleases--;
}
/*
@@ -259,6 +281,7 @@ ipl_update(struct iplease *ipl, uint32_t timeout)
ipl_tdel(ipl);
ipl-ipl_timeout = timeout;
ipl_tadd(ipl);
+lease_save(ipl);
return;
}
@@ -275,8 +298,6 @@ ipl_getbyip(struct iplease *start, uint32_t ipaddr)
return pl;
}
-#define GRACE 5
-
/*
* ipl_trun - run the IP lease timeout list
*/
@@ -465,6 +486,19 @@ dhcpopen(const char *intf)
return handle;
}
+/*
+ * snoopReqFree - release a snoop Req
+ */
+static void
+snoopReqFree(struct virNWFilterSnoopReq *req)
+{
+if (req-ifname)
+VIR_FREE(req-ifname);
+if (req-vars)
+virNWFilterHashTableFree(req-vars);
+VIR_FREE(req);
+}
+
static void *
virNWFilterDHCPSnoop(void *req0)
{
@@ -479,12 +513,19 @@ virNWFilterDHCPSnoop(void *req0)
if (!handle)
return 0;
+/* restore any saved leases for this interface */
+snoop_lock();
+lease_restore(req);
+snoop_unlock();
+
ifindex = if_nametoindex(req-ifname);
while (1) {
if (req-die)
break;
+snoop_lock();
ipl_trun(req);
+snoop_unlock();
packet = (struct eth *) pcap_next(handle,hdr);
@@ -494,16 +535,18 @@ virNWFilterDHCPSnoop(void *req0)
continue;
}
+snoop_lock();
dhcpdecode(req, packet, hdr.caplen);
+snoop_unlock();
}
+
+snoop_lock();
/* free all leases */
for (ipl = req-start; ipl; ipl = req-start)
ipl_del(ipl);
+snoop_unlock();
-/* free all req data */
-VIR_FREE(req-ifname);
-virNWFilterHashTableFree(req-vars);
-VIR_FREE(req);
+snoopReqFree(req);
return 0;
}
@@ -518,9 +561,12 @@ virNWFilterDHCPSnoopReq(virConnectPtr conn,
{
struct virNWFilterSnoopReq *req;
+snoop_lock();
req = virHashLookup(SnoopReqs, ifname);
-if (req)
+snoop_unlock();
+if (req) {
return 0;
+}
if (VIR_ALLOC(req) 0) {
virReportOOMError();
return 1;
@@ -533,28 +579,30 @@ virNWFilterDHCPSnoopReq(virConnectPtr conn,
req-ifname = strdup(ifname);
req-vars = virNWFilterHashTableCreate(0);
if (!req-vars) {
+snoopReqFree(req);
Following the lookup into the hashtable above you cannot just free the
request. I suppose you'd first have to remove it from the hash
Re: [libvirt] Compiler warnings
On 10/17/2011 11:03 AM, Eric Blake wrote: On 10/17/2011 10:17 AM, Alex Jia wrote: Eric, It's latest libvirt upstream, current commit id is commit 0a71c79. # rpm -q glibc glibc-2.12-1.42.el6.x86_64 430 if (strchr(toescape, *cur)) CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Something's screwy. There's no in that line, unless strchr() is a macro based on your particular compilation flags. I'm suspecting that this is a case of -D_FORTIFY_SOURCE going awry. I don't see how this could possibly be a libvirt issue; more likely, it is an issue with the choice of compiler flags you are using, coupled with an issue in the system headers causing the preprocessor to expand strchr() into something that tickles a spurious gcc warning. Just to make sure, could you show 'make V=1' output, so we know exactly which compiler flags are in effect? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Compiler warnings
On 10/17/2011 10:17 AM, Alex Jia wrote: Eric, It's latest libvirt upstream, current commit id is commit 0a71c79. # rpm -q glibc glibc-2.12-1.42.el6.x86_64 430 if (strchr(toescape, *cur)) CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Something's screwy. There's no in that line, unless strchr() is a macro based on your particular compilation flags. I'm suspecting that this is a case of -D_FORTIFY_SOURCE going awry. I don't see how this could possibly be a libvirt issue; more likely, it is an issue with the choice of compiler flags you are using, coupled with an issue in the system headers causing the preprocessor to expand strchr() into something that tickles a spurious gcc warning. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy DROP
Stefan Berger stef...@linux.vnet.ibm.com wrote on 10/17/2011 08:50:14 AM: I agree with Daniel's previous comments that this could introduce compatibility problems. It would be best not to change it or if really need be later on introduce an XML attribute for a chain that allows to choose whether the default policy is accept or drop. This is not a functional change. With the multiple address support, literally all of the chains in question end with -j DROP. By changing the default policy to DROP, it removes the requirement to have the -j DROP _at_the_end_, which makes appending new rules without reference to a priority easier. I think the real sticking point here is whether we consider the existing standard chains as immutable. If, for compatibility's sake, we keep address matching as: if ($IP != ADDRESS) DROP ACCEPT then we cannot support multiple IP addresses. If we change this to: if ($IP == ADDRESS1) RETURN if ($IP == ADDRESS2) RETURN ... DROP all of the standard chains end in -j DROP and making the policy be DROP expresses exactly the same thing with one less rule per chain. This also allows appending more allowed addresses without using a priority to ensure that -j DROP is last. Any customization to the old filters that do DROP or ACCEPT will still work and RETURN and CONTINUE were not supported before, so the default, whether done by -j DROP/ACCEPT or the policy, don't matter-- the custom rule will behave as it does now. It's just that that DROP/ACCEPT will bypass subsequent checks that are now possible if doing RETURN/CONTINUE instead. But, of course, *any* change to the standard filters requires a review of custom filters that link into them or modify them. I think maintaining this level of compatibility simply means we cannot support multiple addresses. Doing that obviously requires changing the existing address matching filters, which therefore can't be linked in in identically the same way. However, I think any existing customization is trivial to apply after, too. Perhaps an example filter that causes a problem? +-DLS -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification
Stefan Berger stef...@linux.vnet.ibm.com wrote on 10/17/2011 09:07:12 AM: On 10/12/2011 03:50 PM, David L Stevens wrote: This patch adds the internal capability to add rules to existing chains instead of using temporary chains and to generate placeholders for chains that are referenced without generating a rule for them immediately. Finally, it includes variable matching for filter instantiation (i.e., instantiate only when a given variable is present in a filter, or only when it is not). Following the above I am not sure what this will be used for as part of this extension. This is used to add rules to existing chains when a new IP address is discovered (i.e., a DHCP ACK from a server occurs). The existing code builds the entire chain as a temporary chain and then swaps it in, which is only appropriate at start-up. For DHCP snooping, we want to add and remove rules that reference IP using a particular value (the address for the ACK or lease expiration) without affecting other rules that don't reference IP or have a different address value. removeRules was already there, but addRules was not. +-DLS -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification
On 10/17/2011 01:23 PM, David Stevens wrote: Stefan Bergerstef...@linux.vnet.ibm.com wrote on 10/17/2011 09:07:12 AM: On 10/12/2011 03:50 PM, David L Stevens wrote: This patch adds the internal capability to add rules to existing chains instead of using temporary chains and to generate placeholders for chains that are referenced without generating a rule for them immediately. Finally, it includes variable matching for filter instantiation (i.e., instantiate only when a given variable is present in a filter, or only when it is not). Following the above I am not sure what this will be used for as part of this extension. This is used to add rules to existing chains when a new IP address is discovered (i.e., a DHCP ACK from a server occurs). The existing code builds the entire chain as a temporary chain and then swaps it in, which is only appropriate at start-up. For DHCP snooping, we want to add and remove rules that reference IP using a particular value (the address for the ACK or lease expiration) without affecting other rules that don't reference IP or have a different address value. removeRules was already there, but addRules was not. Yes, then I understood this correctly. See the other mails regarding the problems I am seeing with it. If there was a way to figure out at what position to insert a rule into an existing chain, i.e. at position 5, rather than always at the end, we could use this addRules() call, otherwise I find it very limiting. Stefan +-DLS -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Do not wait if the PCI device is not managed when reattaching
On 10/17/2011 04:58 AM, Osier Yang wrote: Waiting for qemu-kvm cleaning up the PCI bar(s) mapping with long time while the device is not managed is just waste of time. --- src/qemu/qemu_hostdev.c | 15 --- 1 files changed, 8 insertions(+), 7 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy DROP
On 10/17/2011 01:04 PM, David Stevens wrote: Stefan Bergerstef...@linux.vnet.ibm.com wrote on 10/17/2011 08:50:14 AM: I agree with Daniel's previous comments that this could introduce compatibility problems. It would be best not to change it or if really need be later on introduce an XML attribute for a chain that allows to choose whether the default policy is accept or drop. This is not a functional change. With the multiple address support, literally all of the chains in question end with -j DROP. By changing the default policy to DROP, it removes the requirement to have the -j DROP _at_the_end_, which makes appending new rules without reference to a priority easier. Yes, '_at_the_end_', that's what I thought. I am not sure whether this particular requirement is the best way to proceed since obviously you cannot have any other rules with lesser priority after the ones doing the 'return' -- whatever those rules may be doing. The alternative would be having to instantiate the whole filter chain in the same way as the IP address learning thread does with the 'late' filter instantiation call -- that would at least get all the priorities correct and not introduce a requirement on how one has to write a filter. I think the real sticking point here is whether we consider the existing standard chains as immutable. If, for compatibility's sake, I don't consider them as immutable but should be replaced with something of the same or 'better' functionality. we keep address matching as: if ($IP != ADDRESS) DROP ACCEPT then we cannot support multiple IP addresses. If we change this to: if ($IP == ADDRESS1) RETURN if ($IP == ADDRESS2) RETURN ... DROP all of the standard chains end in -j DROP and making the policy be DROP expresses exactly the same thing with one less rule per chain. This also allows appending more allowed addresses without using a priority to ensure that -j DROP is last. Yes, I understand that but I don't necessarily like the implications of this. Any customization to the old filters that do DROP or ACCEPT will still work and RETURN and CONTINUE were not supported before, so the default, whether done by -j DROP/ACCEPT or the policy, don't matter-- the custom rule will behave as it does now. It's just that that DROP/ACCEPT will bypass subsequent checks that are now possible if doing RETURN/CONTINUE instead. But, of course, *any* change to the standard filters requires a review of custom filters that link into them or modify them. I think maintaining this level of compatibility simply means we cannot support multiple addresses. Doing that obviously requires changing the existing address matching filters, which therefore can't be linked in in identically the same way. However, I think any existing customization is trivial to apply after, too. Perhaps an example filter that causes a problem? I don't have an example filter myself, but I don't know what other people may have written. An alternative would be to say that the existing filters work with the IP address learning thread and we would need to introduce new filters for support of multiple IP addresses. Yes, the current filters aren't prepare for supporting multiple IP addresses per interface. I don't think replacing the existing filters would be a problem per-se, but I don't like that the priority of the rules doing the 'RETURN' is assumed to be the lowest in the chain and we can just append anything to the end of the chain -- the filters we are writing are just examples and someone may come up with a few rules that do something with packets that were not RETURN'ed and thus needs to have rules executing behind those. Again, maybe (the less efficient but more generic way of) instantiating the filters by calling the virNWFilterInstantiateFilterLate() could solve part of the problem. Stefan +-DLS -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Do not reattach PCI device used by other domain when shutdown
On 10/17/2011 04:44 AM, Osier Yang wrote: After thinking a while, even we set the used_by during domains reloading, the other attrs (dev-unbind_from_stub, dev-reprobe, dev-remove_slot) of the PCI dev still will be lost. We can't known what values should be for these attrs as generally they are initialized when do preparation, however the preparations are already passed, and the devices are using by the running domains. And thus the device won't be bound to the original driver (if it had) correctly, or mistakenly bound to some driver but it didn't have one. Perhaps we need some new XMLs introduced for hostdevs. You're right - the only sane way to survive libvirtd restarts is to track more information in the domain xml (it can be internal xml, and doesn't have to be dumped to the user; compare how we track actual as an internal-only detail on how networks were actually allocated). Sounds like we've got more work to do in this area. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 07/10] support variable value changing
Stefan Berger stef...@linux.vnet.ibm.com wrote on 10/17/2011 09:17:21
AM:
+int
+virNWFilterChangeVar(virConnectPtr conn,
+if (virNWFilterHashTablePut(vars, var, value, 1)) {
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _(Cound not
add
+ variable \%s\ to hashmap), var);
+return 1;
+}
+rc = _virNWFilterInstantiateRec(conn,
+techdriver,
+nettype,
+filter,
+ifname,
+vars,
+NWFILTER_STD_VAR_IP, 0,
+nEntries,insts,
+ INSTANTIATE_ALWAYS,foundNewFilter,
+driver);
Given the NWFILTER_STD_VAR_IP parameter, what does it give us at this
point?
I'm not sure I understand the question. NWFILTER_STD_VAR_IP (aka
IP) is
the variable we want to match, so this will only affect rules that
reference IP.
Rules that don't match are not included in the instantiation, and so are
unaffected.
+ if (delete)
+ rc = techdriver-removeRules(conn, ifname, nptrs, ptrs);
+ else
+ rc = techdriver-addRules(conn, ifname, nptrs, ptrs);
I am wondering about this addRules() and whether the rules are being
added to the end of a chain and thus the rules' assumed priority would
have to be such that these rules can actually always be the last ones?
Where they are added depends on the original filter. If they have
a
priority associated with them, it'll use that. It's no different than the
original code's instantiation of rules referencing IP (other than that
this allows multiple occurrences). Without a priority, they appear at the
end of the chain.
+-DLS
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] snapshot: implement LIST_LEAVES flag in esx
On 10/15/2011 04:47 PM, Matthias Bolte wrote: 2011/10/8 Eric Blakeebl...@redhat.com: Relatively straight-forward filtering. * src/esx/esx_vi.h (esxVI_GetNumberOfSnapshotTrees) (esxVI_GetSnapshotTreeNames): Add parameter. * src/esx/esx_vi.c (esxVI_GetNumberOfSnapshotTrees) (esxVI_GetSnapshotTreeNames): Allow leaf filtering. * src/esx/esx_driver.c (esxDomainSnapshotNum) (esxDomainSnapshotListNames, esxDomainSnapshotNumChildren) (esxDomainSnapshotListChildrenNames): Pass new flag through. --- Tested, works, ACK. Thanks; pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification
Stefan Berger stef...@linux.vnet.ibm.com wrote on 10/17/2011 10:31:29 AM: was not. Yes, then I understood this correctly. See the other mails regarding the problems I am seeing with it. If there was a way to figure out at what position to insert a rule into an existing chain, i.e. at position 5, rather than always at the end, we could use this addRules() call, otherwise I find it very limiting. I'm not sure if I answered this already for you or not, but you can -- by using the priority in the rule. If we don't use the policy and so have to have a -j DROP at the end, then we'd want the original filter to use -1 (if I'm remembering correctly -- 1 before end??). You can specify the rule be added at any point; IP rules would all have the same priority, because they originate from the same line in the filter, but you can use the priority to offset from the end or beginning, or any fixed point in the chain. +-DLS PS - I haven't tried using negative priorities with nwfilter, but ebtables/iptables supports it, at least. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for autodestroy of guests to the LXC and UML drivers
On 10/17/2011 04:32 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com I wrote this months ago and thought I had already submitted/merged it. Obviously not. I need this for the virt sandbox tools asap. We recently added support for VIR_DOMAIN_START_AUTODESTROY and an impl to the QEMU driver. It is very desirable to support in other drivers, so this adds it to LXC and UML * src/lxc/lxc_conf.h, src/lxc/lxc_driver.c, src/uml/uml_conf.h, src/uml/uml_driver.c: Wire up autodestroy functions --- src/lxc/lxc_conf.h |5 ++ src/lxc/lxc_driver.c | 140 +-- src/uml/uml_conf.h |6 ++ src/uml/uml_driver.c | 164 +++--- 4 files changed, 289 insertions(+), 26 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Honor the original properties of PCI device when detaching
On 10/17/2011 05:07 AM, Osier Yang wrote:
This patch fixes two problems:
1) The device will be reattached to host even if it's not
managed, as there is a pciDeviceSetManaged.
2) The device won't be reattached to host with original
driver properly. As it doesn't honor the device original
properties which are maintained by driver-activePciHostdevs.
---
src/qemu/qemu_hotplug.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bfa524b..f3f0060 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1956,6 +1956,7 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver
*driver,
qemuDomainObjPrivatePtr priv = vm-privateData;
int i, ret;
pciDevice *pci;
+pciDevice *activePci;
for (i = 0 ; i vm-def-nhostdevs ; i++) {
if (vm-def-hostdevs[i]-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
@@ -2015,16 +2016,15 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver
*driver,
detach-source.subsys.u.pci.bus,
detach-source.subsys.u.pci.slot,
detach-source.subsys.u.pci.function);
-if (!pci)
-ret = -1;
-else {
-pciDeviceSetManaged(pci, detach-managed);
-pciDeviceListDel(driver-activePciHostdevs, pci);
-if (pciResetDevice(pci, driver-activePciHostdevs, NULL) 0)
+if (pci) {
+activePci = pciDeviceListSteal(driver-activePciHostdevs, pci);
+if (pciResetDevice(activePci, driver-activePciHostdevs, NULL) 0)
ret = -1;
-pciDeviceReAttachInit(pci);
-qemuReattachPciDevice(pci, driver);
+qemuReattachPciDevice(activePci, driver);
This calls qemuReattachPciDevice even if pciResetDevice failed. I don't
think that is right.
However, I agree with you that hot-unplug cannot make a device managed
if it was not already managed, and also that it should be using the
device attributes from driver-activePciHostdevs rather than from the
just-created pci in order to properly decide how much work to do on the
unplug event.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification
On 10/17/2011 01:58 PM, David Stevens wrote: Stefan Bergerstef...@linux.vnet.ibm.com wrote on 10/17/2011 10:31:29 AM: was not. Yes, then I understood this correctly. See the other mails regarding the problems I am seeing with it. If there was a way to figure out at what position to insert a rule into an existing chain, i.e. at position 5, rather than always at the end, we could use this addRules() call, otherwise I find it very limiting. I'm not sure if I answered this already for you or not, but you can -- by using the priority in the rule. If we don't use the policy and so have to have a -j DROP at the end, then we'd want the original filter to use -1 (if I'm remembering correctly -- 1 before end??). You can specify the rule be added at any point; IP rules would all have the same priority, because they originate from the same line in the filter, but you can use the priority to offset from the end or beginning, or any fixed point in the chain. +-DLS PS - I haven't tried using negative priorities with nwfilter, but ebtables/iptables supports it, at least. The ebtables / iptables insertion of rules is based on position of the rule relative to other existing rules and has nothing to do with nwfilter priority which servers sorting of rules relative to each other beyond what their occurrence in the XML provides. So the priority doesn't map directly into the position of the rule as ebtables/iptables needs it. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 08/10] add DHCP snooping
On 10/12/2011 03:50 PM, David L Stevens wrote: + +#includeconfig.h + +#ifdef HAVE_LIBPCAP +#includepcap.h +#endif HAVE_LIBPCAP also has to be handled in the parts that actually call the pcap library API and provide a way of failing if libpcap is not available. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt (v2)
The glibc ones cannot handle ptys opened in a devpts not mounted at
/dev/pts.
Changelog:
Oct 17: Per Eli Qiao, use VIR_ALLOC_N when appropriate, make
sure to check return values, and follow coding style
convention.
Change lxcGetTtyGid() to return -1 on error, otherwise
return gid in passed-in argument.
Signed-off-by: Serge Hallyn serge.hal...@canonical.com
---
src/lxc/lxc_controller.c | 89 +++--
1 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 89ce7f5..8c9caee 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -41,6 +41,8 @@
#include locale.h
#include linux/loop.h
#include dirent.h
+#include grp.h
+#include sys/stat.h
#if HAVE_CAPNG
# include cap-ng.h
@@ -781,6 +783,88 @@ static int lxcSetPersonality(virDomainDefPtr def)
#endif
static int
+lxcGetTtyGid(int *gid) {
+char *grtmpbuf;
+struct group grbuf;
+size_t grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX);
+struct group *p;
+int ret;
+gid_t tty_gid = -1;
+
+/* Get the group ID of the special `tty' group. */
+if (grbuflen == (size_t) -1L)
+/* `sysconf' does not support _SC_GETGR_R_SIZE_MAX.
+ Try a moderate value. */
+grbuflen = 1024;
+
+if ((VIR_ALLOC_N(grtmpbuf, grbuflen)) 0)
+ return -1;
+
+ret = getgrnam_r(tty, grbuf, grtmpbuf, grbuflen, p);
+if (ret || p != NULL)
+tty_gid = p-gr_gid;
+
+VIR_FREE(grtmpbuf);
+
+*gid = tty_gid == -1 ? getgid() : tty_gid;
+return 0;
+}
+
+#define ACCESSPERMS (S_IRWXU|S_IRWXG|S_IRWXO) /* 0777 */
+
+/* heavily borrowed from glibc, but don't assume devpts == /dev/pts */
+static int
+lxcCreateTty(char *ptmx, int *ttymaster, char **ttyName)
+{
+int rc = -1;
+int ret;
+int ptyno;
+uid_t uid;
+gid_t gid;
+struct stat st;
+int unlock = 0;
+
+if ((*ttymaster = open(ptmx, O_RDWR|O_NOCTTY|O_NONBLOCK)) 0)
+goto cleanup;
+
+if (ioctl(*ttymaster, TIOCSPTLCK, unlock) 0)
+goto cleanup;
+
+if (ioctl(*ttymaster, TIOCGPTN, ptyno) 0)
+goto cleanup;
+
+if (fstat(*ttymaster, st) 0)
+goto cleanup;
+
+if (lxcGetTtyGid(gid) 0)
+goto cleanup;
+
+uid = getuid();
+if (st.st_uid != uid || st.st_gid != gid)
+if (fchown(*ttymaster, uid, gid) 0)
+goto cleanup;
+
+if ((st.st_mode ACCESSPERMS) != (S_IRUSR|S_IWUSR|S_IWGRP))
+if (fchmod(*ttymaster, S_IRUSR|S_IWUSR|S_IWGRP) 0)
+goto cleanup;
+
+if (VIR_ALLOC_N(*ttyName, PATH_MAX) 0) {
+errno = ENOMEM;
+goto cleanup;
+}
+
+snprintf(*ttyName, PATH_MAX, /dev/pts/%d, ptyno);
+
+rc = 0;
+
+cleanup:
+if (rc != 0)
+VIR_FORCE_CLOSE(*ttymaster);
+
+return rc;
+}
+
+static int
lxcControllerRun(virDomainDefPtr def,
unsigned int nveths,
char **veths,
@@ -894,10 +978,7 @@ lxcControllerRun(virDomainDefPtr def,
if (devptmx) {
VIR_DEBUG(Opening tty on private %s, devptmx);
-if (virFileOpenTtyAt(devptmx,
- containerPty,
- containerPtyPath,
- 0) 0) {
+if (lxcCreateTty(devptmx, containerPty, containerPtyPath) 0) {
virReportSystemError(errno, %s,
_(Failed to allocate tty));
goto cleanup;
--
1.7.5.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
Adam Litke wrote: On Mon, Oct 03, 2011 at 10:46:18PM -0600, Jim Fehlig wrote: Adam Litke wrote: Hi Jim. I was testing this and found that I could not boot from the sata disks I defined. When I switch them back to ide, they can be booted just fine. Perhaps something is missing from the boot order logic? Hmm, I didn't notice this in my testing. sda in the below config contained /boot. Can you provide the domain config? Hi Adam, I've been traveling, sorry for the delay... With the following config, I end up booting from the cdrom. If I take away the cdrom definition completely, I am unable to boot at all. I can boot fine with your config, either from sda (boot dev='hd' /) or cdrom (boot dev='cdrom' /). Have you verified your seabios supports booting from ahci as Bruce mentioned? Thanks, Jim domain type='kvm' nameblockPull-test/name memory262144/memory currentMemory262144/currentMemory vcpu1/vcpu os type arch='x86_64' machine='pc-0.13'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashrestart/on_crash devices emulator/home/aglitke/src/qemu/x86_64-softmmu/qemu-system-x86_64/emulator disk type='file' device='disk' driver name='qemu' type='qed'/ source file='/home/aglitke/vm/sata-test/disk1.qed' / target dev='sda' bus='sata'/ /disk disk type='file' device='disk' driver name='qemu' type='qed'/ source file='/home/aglitke/vm/sata-test/disk2.qed' / target dev='sdb' bus='sata'/ /disk disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/home/aglitke/vm/images/natty-alternate-amd64.iso' / target dev='hda' bus='ide'/ /disk interface type=network source network=default / /interface graphics type='vnc' port='-1' autoport='yes'/ /devices /domain -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
On 09/28/2011 04:43 PM, Jim Fehlig wrote: Tested with multiple AHCI controllers and multiple disks attached to a controller. E.g., +++ b/src/qemu/qemu_capabilities.c @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, no-shutdown, cache-unsafe, /* 75 */ + ich9-ahci, You'll have to rebase here, but nothing too difficult. ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt (v2)
On 10/17/2011 01:04 PM, Serge E. Hallyn wrote:
The glibc ones cannot handle ptys opened in a devpts not mounted at
/dev/pts.
Changelog:
Oct 17: Per Eli Qiao, use VIR_ALLOC_N when appropriate, make
sure to check return values, and follow coding style
convention.
Change lxcGetTtyGid() to return -1 on error, otherwise
return gid in passed-in argument.
Signed-off-by: Serge Hallynserge.hal...@canonical.com
---
src/lxc/lxc_controller.c | 89 +++--
1 files changed, 85 insertions(+), 4 deletions(-)
@@ -781,6 +783,88 @@ static int lxcSetPersonality(virDomainDefPtr def)
#endif
static int
+lxcGetTtyGid(int *gid) {
+char *grtmpbuf;
+struct group grbuf;
+size_t grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX);
+struct group *p;
+int ret;
+gid_t tty_gid = -1;
Hmm. This gets called once per lxc guest, instead of once per libvirtd
process, even though the gid won't change in the meantime.
Furthermore, we have _already_ hardcoded this to 5, based on the options
we gave to mount(devpts) - either we need to fix that mount call
(better), or we can skip this function altogether (assuming that our
hard-coding of 5 is correct, there's no need to requery it, although
that does sound like a worse solution). For that matter, the whole
point of the mount(devpts,...,gid=5) designation is that the new pty
will be owned by gid 5, without needing to fchown() it. Are there
kernels that are new enough to support newinstance mounting, yet old
enough to not honor gid=5? That would be the only case where we would
have to chown in the first place. But if all kernels new enough to
support newinstance mounting correctly honor the gid=5 option, then we
don't even have to do chown() calls [but we still have to fix the
hard-coding of gid=5 in the mount() option].
+if (fstat(*ttymaster,st) 0)
+goto cleanup;
+
+if (lxcGetTtyGid(gid) 0)
+goto cleanup;
+
+uid = getuid();
+if (st.st_uid != uid || st.st_gid != gid)
+if (fchown(*ttymaster, uid, gid) 0)
+goto cleanup;
+
+if ((st.st_mode ACCESSPERMS) != (S_IRUSR|S_IWUSR|S_IWGRP))
+if (fchmod(*ttymaster, S_IRUSR|S_IWUSR|S_IWGRP) 0)
+goto cleanup;
Likewise, I think this fchmod() is useless; the mode=0620 in the mount
option should have already set this up.
+
+if (VIR_ALLOC_N(*ttyName, PATH_MAX) 0) {
+errno = ENOMEM;
+goto cleanup;
+}
Wasteful. PATH_MAX is MUCH bigger than we need.
+
+snprintf(*ttyName, PATH_MAX, /dev/pts/%d, ptyno);
Instead, I'd just do this as:
virAsprintf(ttyName, /dev/pts/%d, ptyno);
Also, do we want this to be the name of the pty, _as seen from the guest
after the fs pivot is complete_, or do we want this to be the name of
the pty, as seen from the host prior to the pivot, in which case it
would be some derivative of %s/dev/pts/%d, root-src, ptyno?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy DROP
Stefan Berger stef...@linux.vnet.ibm.com wrote on 10/17/2011 10:29:08 AM: Yes, '_at_the_end_', that's what I thought. I am not sure whether this particular requirement is the best way to proceed since obviously you cannot have any other rules with lesser priority after the ones doing the 'return' -- whatever those rules may be doing. Not in the same chain, but if a chain is checking multiple allowed values for the same field, that's all that chain should be doing. Checking some other condition should be done after passing, e.g., the IP address checks and would be done because of the RETURN. Current code does an ACCEPT or REJECT at that point, so *requires* modification of the existing chain. Any current filters are even worse because without RETURN and CONTINUE, they can only accept or drop a packet without further processing. With this patchset, you can apply multiple tests to the same packet with only the restriction that testing other fields in that packet require a different subchain-- something not possible at all today. If, for example, you want to allow 100 ports and a particular IP address, reject everything else, won't that require a DROP rule for 65436 ports so that you don't accept either based on just the port or just the IP address? Or , you'd have to combine the IP address check in every port-check rule and do it before you do the blanket ACCEPT in the standard rule. An alternative would be to say that the existing filters work with the IP address learning thread and we would need to introduce new filters for support of multiple IP addresses. Yes, the current filters aren't prepare for supporting multiple IP addresses per interface. Yes, I thought about this, but it really is just duplicating the entire set with a different name. I think especially without support for RETURN/CONTINUE, as a practical matter the only way to modify the current filters to do interesting things is to replace them. Anyone doing that will not be affected by a change to the standard rules, except for the possibly trivial addition of a -j ACCEPT at the end if they require a default ACCEPT for the modified rules. I don't think replacing the existing filters would be a problem per-se, but I don't like that the priority of the rules doing the 'RETURN' is assumed to be the lowest in the chain and we can just append anything to the end of the chain -- the filters we are writing are just examples and someone may come up with a few rules that do something with packets that were not RETURN'ed and thus needs to have rules executing behind those. Again, maybe (the less efficient but more generic way of) instantiating the filters by calling the virNWFilterInstantiateFilterLate() could solve part of the problem. The only use I've added for addRules is the multiple address check and that should not have other random fields in the same chain. Better would be to have a different chain linked at the top after the address checks. In fact, for compatibility, it'd be possible to change the address checks to a different chain and leave an empty ip and arp chain that does only accept. It's just that it isn't really all that compatible if what's really happening is that a customer is replacing the existing chains with modifications, rather than applying additional rules to it. In the end, any modification whatsoever to the examples directory requires someone who has customized a filter to look at the customization again. Addition of RETURN/CONTINUE makes that easier in the future. With the existing filters, any customization cannot be independent of the internals of the existing filters (it's adding rules in the middle of it), so the only way to maintain compatibility is to never change them. With RETURN/CONTINUE and your suggested user-defined chain(s), you can apply the standard sets with RETURN for acceptable packets and then apply any user-defined checks also to the same packets afterwards. I think that's the right approach, but it requires either changing the existing chains or introducing a completely parallel set and not using the original filters at all, except for sites that want the old stuff only. +-DLS -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt (v2)
Quoting Eric Blake (ebl...@redhat.com):
On 10/17/2011 01:04 PM, Serge E. Hallyn wrote:
The glibc ones cannot handle ptys opened in a devpts not mounted at
/dev/pts.
Changelog:
Oct 17: Per Eli Qiao, use VIR_ALLOC_N when appropriate, make
sure to check return values, and follow coding style
convention.
Change lxcGetTtyGid() to return -1 on error, otherwise
return gid in passed-in argument.
Signed-off-by: Serge Hallynserge.hal...@canonical.com
---
src/lxc/lxc_controller.c | 89
+++--
1 files changed, 85 insertions(+), 4 deletions(-)
@@ -781,6 +783,88 @@ static int lxcSetPersonality(virDomainDefPtr def)
#endif
static int
+lxcGetTtyGid(int *gid) {
+char *grtmpbuf;
+struct group grbuf;
+size_t grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX);
+struct group *p;
+int ret;
+gid_t tty_gid = -1;
Hmm. This gets called once per lxc guest, instead of once per
libvirtd process, even though the gid won't change in the meantime.
Furthermore, we have _already_ hardcoded this to 5, based on the
options we gave to mount(devpts) - either we need to fix that
mount call (better), or we can skip this function altogether
(assuming that our hard-coding of 5 is correct, there's no need to
requery it, although that does sound like a worse solution). For
that matter, the whole point of the mount(devpts,...,gid=5)
designation is that the new pty will be owned by gid 5, without
needing to fchown() it. Are there kernels that are new enough to
support newinstance mounting, yet old enough to not honor gid=5?
No. Mode and gid precede multiple devpts instances.
That would be the only case where we would have to chown in the
first place. But if all kernels new enough to support newinstance
mounting correctly honor the gid=5 option, then we don't even have
to do chown() calls [but we still have to fix the hard-coding of
gid=5 in the mount() option].
I missed something - why do we have to fix that?
It seems to me you're right - this is a linux-specific fn and
we are setting gid to 5 and the mode, we can skip this whole lxcGetTtyGid
function and the corresponding fchown and fchmod calls. Nice!
+if (fstat(*ttymaster,st) 0)
+goto cleanup;
+
+if (lxcGetTtyGid(gid) 0)
+goto cleanup;
+
+uid = getuid();
+if (st.st_uid != uid || st.st_gid != gid)
+if (fchown(*ttymaster, uid, gid) 0)
+goto cleanup;
+
+if ((st.st_mode ACCESSPERMS) != (S_IRUSR|S_IWUSR|S_IWGRP))
+if (fchmod(*ttymaster, S_IRUSR|S_IWUSR|S_IWGRP) 0)
+goto cleanup;
Likewise, I think this fchmod() is useless; the mode=0620 in the
mount option should have already set this up.
Yup.
+
+if (VIR_ALLOC_N(*ttyName, PATH_MAX) 0) {
+errno = ENOMEM;
+goto cleanup;
+}
Wasteful. PATH_MAX is MUCH bigger than we need.
I thought so, but it was being done that way before, and didn't
seem all that important.
+
+snprintf(*ttyName, PATH_MAX, /dev/pts/%d, ptyno);
Instead, I'd just do this as:
virAsprintf(ttyName, /dev/pts/%d, ptyno);
Where virAsprintf will allocate when ttyName starts as null?
Also, do we want this to be the name of the pty, _as seen from the
guest after the fs pivot is complete_, or do we want this to be the
name of the pty, as seen from the host prior to the pivot, in which
case it would be some derivative of %s/dev/pts/%d, root-src,
ptyno?
This gets passed into lxc_container which then prepends the chroot
location. Don't know if there is any reason why we can't just
set it to the full name here, haven't looked further.
-serge
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt (v2)
On 10/17/2011 03:29 PM, Serge E. Hallyn wrote: that matter, the whole point of the mount(devpts,...,gid=5) designation is that the new pty will be owned by gid 5, without needing to fchown() it. Are there kernels that are new enough to support newinstance mounting, yet old enough to not honor gid=5? No. Mode and gid precede multiple devpts instances. Good to hear. In other words, glibc has to worry about it when dealing with /dev/pts, because it caters to older kernels that lacked multiple devpts, but lxc can take shortcuts based on guarantees present by virtue of the existence of multiple instances. But definitely some comments in the code are called for, explaining why we are taking these shortcuts. [but we still have to fix the hard-coding of gid=5 in the mount() option]. I missed something - why do we have to fix that? We don't have to fix it now, but we should fix it someday. There's nothing that says a distro has to map 'tty' to gid 5, and while most distros do that, we should instead be portable to compilation on a distro where 'tty' is gid 6 (or some other unusual number). Instead, I'd just do this as: virAsprintf(ttyName, /dev/pts/%d, ptyno); Where virAsprintf will allocate when ttyName starts as null? Yep - the whole point of virAsprintf is to allocate the string on your behalf, and to gracefully ensure that ttyName is NULL on allocation failure - much more compact than using snprintf yourself, and avoids the waste of a PATH_MAX allocation. And, while you are at it, what about also fixing src/util/util.c to remove virFileOpenTtyAt (now that no one calls that), by moving its body into virFileOpenTty except for using posix_openpt instead of open(/dev/ptmx). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add AHCI support to qemu driver
Eric Blake wrote: On 09/28/2011 04:43 PM, Jim Fehlig wrote: Tested with multiple AHCI controllers and multiple disks attached to a controller. E.g., +++ b/src/qemu/qemu_capabilities.c @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, no-shutdown, cache-unsafe, /* 75 */ + ich9-ahci, You'll have to rebase here, but nothing too difficult. Yep, simple rebase ACK. and pushed. Thanks! Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy DROP
On 10/17/2011 05:22 PM, David Stevens wrote: Stefan Bergerstef...@linux.vnet.ibm.com wrote on 10/17/2011 10:29:08 AM: Yes, '_at_the_end_', that's what I thought. I am not sure whether this particular requirement is the best way to proceed since obviously you cannot have any other rules with lesser priority after the ones doing the 'return' -- whatever those rules may be doing. Not in the same chain, but if a chain is checking multiple allowed values for the same field, that's all that chain should be doing. Sure, and what if I wanted to do something with packets that were not RETURN'ed, where would I treat them besides *after* the RETURN? Checking some other condition should be done after passing, e.g., the IP address checks and would be done because of the RETURN. Current code does an ACCEPT or REJECT at that point, so *requires* modification of the existing chain. Any current filters are even worse because without RETURN and CONTINUE, they can only accept or drop a packet without further processing. Not arguing against that. With this patchset, you can apply multiple tests to the same packet with only the restriction that testing other fields in that packet require a different subchain-- something not possible at all today. If, for example, you want to allow 100 ports and a particular IP address, reject everything else, won't that require a DROP rule for 65436 ports so that you don't accept either based on just the port or just the IP address? Or , you'd have to combine the IP address check in every port-check rule and do it before you do the blanket ACCEPT in the standard rule. An alternative would be to say that the existing filters work with the IP address learning thread and we would need to introduce new filters for support of multiple IP addresses. Yes, the current filters aren't prepare for supporting multiple IP addresses per interface. Yes, I thought about this, but it really is just duplicating the entire set with a different name. I think especially without support for RETURN/CONTINUE, as a practical matter the only way to modify the current filters to do interesting things is to replace them. Anyone doing that will not be affected by a change to the standard rules, except for the possibly trivial addition of a -j ACCEPT at the end if they require a default ACCEPT for the modified rules. I don't think replacing the existing filters would be a problem per-se, but I don't like that the priority of the rules doing the 'RETURN' is assumed to be the lowest in the chain and we can just append anything to the end of the chain -- the filters we are writing are just examples and someone may come up with a few rules that do something with packets that were not RETURN'ed and thus needs to have rules executing behind those. Again, maybe (the less efficient but more generic way of) instantiating the filters by calling the virNWFilterInstantiateFilterLate() could solve part of the problem. The only use I've added for addRules is the multiple address check and that should not have other random fields in the same chain. Better would be to have a different chain linked at the What if in the future we were to support ebtables logging and wanted to log any packet that has not been handled with a RETURN, thus for example logging MAC spoofing -- just as an example. I don't see how one would do that without adding rules into the mac chain *after* the rules that do the RETURN for packets with acceptable source MAC addresses and all others get logged. I would not call these 'random fields' being evaluated, but that the implementation of addRules() requires the filters to be limited in a way that may at some point become a problem for some use cases and someone has to reimplement it then. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv3 10/10] support multiple static IP addresses
On 10/12/2011 03:50 PM, David L Stevens wrote:
This patch adds support for multiple static IP addresses in a
comma-separated list. For example:
interface type='network'
filterref filter='clean-traffic'
parameter name='ip_learning' value='none'/
parameter name='IP' value='10.0.0.1,10.1.0.7,10.0.3.8,10.0.9.244'/
/filterref
...
Signed-off-by: David L Stevensdlstev...@us.ibm.com
---
src/nwfilter/nwfilter_gentech_driver.c | 26 ++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/src/nwfilter/nwfilter_gentech_driver.c
b/src/nwfilter/nwfilter_gentech_driver.c
index 577b48d..8f74a01 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -920,6 +920,8 @@ __virNWFilterInstantiateFilter(virConnectPtr conn,
char *str_macaddr = NULL;
const char *ipaddr;
char *str_ipaddr = NULL;
+char *ipaddrlist = NULL;
+char *sep;
techdriver = virNWFilterTechDriverForName(drvname);
@@ -983,6 +985,17 @@ __virNWFilterInstantiateFilter(virConnectPtr conn,
goto err_exit_vars1;
}
+ipaddr = virHashLookup(filterparams-hashTable, NWFILTER_STD_VAR_IP);
+if (ipaddr) {
+sep = strchr(ipaddr, ',');
+if (sep) {
+str_ipaddr = strndup(ipaddr, sep-ipaddr);
+ipaddrlist = strdup(sep + 1);
Check for str_ipaddr or ipaddrlist being NULL.
+virNWFilterHashTablePut(vars, NWFILTER_STD_VAR_IP, str_ipaddr, 1);
+}
+}
+str_ipaddr = NULL;
+
filter = obj-def;
switch (useNewFilter) {
@@ -1011,6 +1024,19 @@ __virNWFilterInstantiateFilter(virConnectPtr conn,
driver,
forceWithPendingReq);
+/* add the rest of the IP list */
+for (ipaddr = ipaddrlist; ipaddr; ipaddr = sep) {
+sep = strchr(ipaddr, ',');
+if (sep) {
+str_ipaddr = strndup(ipaddr, sep-ipaddr);
Test for NULL result.
+sep++; /* skip ',' */
+} else
+str_ipaddr = strdup(ipaddr);
Also here.
+virNWFilterChangeVar(conn, techdriver, nettype, filter, ifname, vars,
+ driver, NWFILTER_STD_VAR_IP, str_ipaddr, 0);
+
+}
+str_ipaddr = NULL;
virNWFilterHashTableFree(vars);
err_exit_vars1:
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Do not wait if the PCI device is not managed when reattaching
于 2011年10月18日 01:47, Eric Blake 写道: On 10/17/2011 04:58 AM, Osier Yang wrote: Waiting for qemu-kvm cleaning up the PCI bar(s) mapping with long time while the device is not managed is just waste of time. --- src/qemu/qemu_hostdev.c | 15 --- 1 files changed, 8 insertions(+), 7 deletions(-) ACK. Pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Compiler warnings
At 10/18/2011 01:04 AM, Eric Blake Write: On 10/17/2011 11:03 AM, Eric Blake wrote: On 10/17/2011 10:17 AM, Alex Jia wrote: Eric, It's latest libvirt upstream, current commit id is commit 0a71c79. # rpm -q glibc glibc-2.12-1.42.el6.x86_64 430 if (strchr(toescape, *cur)) CC libvirt_util_la-buf.lo util/buf.c: In function 'virBufferEscape': util/buf.c:430: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] Something's screwy. There's no in that line, unless strchr() is a macro based on your particular compilation flags. I'm suspecting that this is a case of -D_FORTIFY_SOURCE going awry. I don't see how this could possibly be a libvirt issue; more likely, it is an issue with the choice of compiler flags you are using, coupled with an issue in the system headers causing the preprocessor to expand strchr() into something that tickles a spurious gcc warning. Just to make sure, could you show 'make V=1' output, so we know exactly which compiler flags are in effect? I can reproduce this problem, here is 'make V=1' output: = make[3]: Entering directory `/root/rpmbuild/BUILD/libvirt-0.9.6/src' /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../src/util -I../include -DIN_LIBVIRT-I/usr/include/libxml2 -Wall -W -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op -Waggregate-return -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization -Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar -Wno-missing-field-initializers -Wno-sign-compare -Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables -fdiagnostics-s how-option -funit-at-a-time -Werror -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -MT libvirt_util_la-buf.lo -MD -MP -MF .deps/libvirt_util_la-buf.Tpo -c -o libvirt_util_la-buf.lo `test -f 'util/buf.c' || echo './'`util/buf.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../src/util -I../include -DIN_LIBVIRT -I/usr/include/libxml2 -Wall -W -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op -Waggregate-return -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization -Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar -Wno-missing-field-initializers -Wno-sign-compare -Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time -Werror -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -MT libvirt_util_la-buf.lo -MD -MP -MF .deps/libvirt_util_la-buf.Tpo -c util/buf.c -fPIC -DPIC -o .libs/libvirt_util_la-buf.o cc1: warnings being treated as errors util/buf.c: In function 'virBufferEscape': util/buf.c:430: error: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] make[3]: *** [libvirt_util_la-buf.lo] Error 1 = If I remove -O2, I can build util/buf.c. I am still investigating this problem. Thanks Wen Congyang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 01/13] virbuf: fix const-correctness
On 10/17/2011 10:36 PM, Eric Blake wrote:
On 10/17/2011 05:20 AM, Hai Dong Li wrote:
On 09/30/2011 12:22 AM, Eric Blake wrote:
Although the compiler wasn't complaining (since it was the pointer,
rather than what was being pointed to, that was actually const), it
looks quite suspicious to call a function with an argument labeled
const when the nature of the pointer (virBufferPtr) is hidden behind
a typedef. Dropping const makes the function declarations easier
to read.
+++ b/src/util/buf.c
@@ -39,7 +39,7 @@ struct _virBuffer {
* freeing the content and setting the error flag.
*/
static void
-virBufferSetError(const virBufferPtr buf)
+virBufferSetError(virBufferPtr buf)
{
VIR_FREE(buf-content);
buf-size = 0;
@@ -113,7 +113,7 @@ virBufferAdd(const virBufferPtr buf, const char
*str, int len)
Maybe a typo here?The virBufferPtr still remains const, but the
declaration of this function in the buf.h shows this const is
removed, too.
No. Rather, this is an artifact of git diff. When showing the
context line for a hunk, it uses the context from the pre-patch file
(where the 'const' was still present), even though the hunk itself
applies to the post-patch version without 'const'. Basically, I never
worry about the @@ lines - they are there as hints for where to apply
the patch, and not a definitive part of the patch itself.
Okay, forget the context line.
What I wanted to say is that the function virBufferAdd in the buf.c
still has the const before the virBufferPtr while the declaration of
this function in buf.h has const removed, after applying this patch.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Compiler warnings
At 10/18/2011 01:03 AM, Eric Blake Write:
On 10/17/2011 10:17 AM, Alex Jia wrote:
Eric,
It's latest libvirt upstream, current commit id is commit 0a71c79.
# rpm -q glibc
glibc-2.12-1.42.el6.x86_64
430 if (strchr(toescape, *cur))
CC libvirt_util_la-buf.lo
util/buf.c: In function 'virBufferEscape':
util/buf.c:430: warning: logical '' with non-zero constant will
always
evaluate as true [-Wlogical-op]
Something's screwy. There's no in that line, unless strchr() is a
macro based on your particular compilation flags. I'm suspecting that
this is a case of -D_FORTIFY_SOURCE going awry. I don't see how this
could possibly be a libvirt issue; more likely, it is an issue with the
choice of compiler flags you are using, coupled with an issue in the
system headers causing the preprocessor to expand strchr() into
something that tickles a spurious gcc warning.
Yes, strchr() is a macro, I use -E, and get the following:
while (*cur != 0) {
if ((__extension__ (__builtin_constant_p (*cur)
!__builtin_constant_p (toescape) (*cur) == '\0' ? (char *) __rawmemchr
(toescape, *cur) : __builtin_strchr (toescape, *cur
*out++ = '\\';
*out++ = *cur;
cur++;
}
*out = 0;
Thanks
Wen Congyang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Compiler warnings
At 10/18/2011 09:32 AM, Wen Congyang Write:
At 10/18/2011 01:04 AM, Eric Blake Write:
On 10/17/2011 11:03 AM, Eric Blake wrote:
On 10/17/2011 10:17 AM, Alex Jia wrote:
Eric,
It's latest libvirt upstream, current commit id is commit 0a71c79.
# rpm -q glibc
glibc-2.12-1.42.el6.x86_64
430 if (strchr(toescape, *cur))
CC libvirt_util_la-buf.lo
util/buf.c: In function 'virBufferEscape':
util/buf.c:430: warning: logical '' with non-zero constant will
always
evaluate as true [-Wlogical-op]
Something's screwy. There's no in that line, unless strchr() is a
macro based on your particular compilation flags. I'm suspecting that
this is a case of -D_FORTIFY_SOURCE going awry. I don't see how this
could possibly be a libvirt issue; more likely, it is an issue with the
choice of compiler flags you are using, coupled with an issue in the
system headers causing the preprocessor to expand strchr() into
something that tickles a spurious gcc warning.
Just to make sure, could you show 'make V=1' output, so we know exactly
which compiler flags are in effect?
I can reproduce this problem, here is 'make V=1' output:
=
make[3]: Entering directory `/root/rpmbuild/BUILD/libvirt-0.9.6/src'
/bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H
-I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../src/util
-I../include -DIN_LIBVIRT-I/usr/include/libxml2 -Wall -W
-Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused
-Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith
-Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op
-Waggregate-return -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand
-Wattributes -Wcoverage-mismatch -Wmultichar -Wno-missing-field-initializers
-Wno-sign-compare -Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions
-fasynchronous-unwind-tables -fdiagnostics
-s
how-option -funit-at-a-time -Werror -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT libvirt_util_la-buf.lo -MD
-MP -MF .deps/libvirt_util_la-buf.Tpo -c -o libvirt_util_la-buf.lo `test -f
'util/buf.c' || echo './'`util/buf.c
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib
-I../gnulib/lib -I../include -I../src/util -I../include -DIN_LIBVIRT
-I/usr/include/libxml2 -Wall -W -Wformat-y2k -Wformat-security -Winit-self
-Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow
-Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op
-Waggregate-return -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand
-Wattributes -Wcoverage-mismatch -Wmultichar -Wno-missing-field-initializers
-Wno-sign-compare -Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time -Werro
r
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT libvirt_util_la-buf.lo -MD
-MP -MF .deps/libvirt_util_la-buf.Tpo -c util/buf.c -fPIC -DPIC -o
.libs/libvirt_util_la-buf.o
cc1: warnings being treated as errors
util/buf.c: In function 'virBufferEscape':
util/buf.c:430: error: logical '' with non-zero constant will always
evaluate as true [-Wlogical-op]
make[3]: *** [libvirt_util_la-buf.lo] Error 1
=
If I remove -O2, I can build util/buf.c.
I test strchr in a small program:
=
# cat test.c
#include stdio.h
#include string.h
int main() {
char *s = abcdefg;
int chr = (int)'d';
char *d;
d = strchr(s, chr);
if (d)
printf(%s\n, d);
return 0;
}
# gcc -std=c99 -Wall -o test test.c -Wlogical-op -Werror
# gcc -std=c99 -O2 -Wall -o test test.c -Wlogical-op -Werror
cc1: warnings being treated as errors
test.c: In function ‘main’:
test.c:9: error: logical ‘’ with non-zero constant will always evaluate as
true
# gcc -std=c99 -Wall -o test test.c -Wlogical-op -Werror
# gcc -std=c99 -Wall -o test test.c -Werror
=
The building failed only when '-O2' and '-Wlogical-op' are specified at the
Re: [libvirt] Compiler warnings
On 10/18/2011 10:09 AM, Wen Congyang wrote:
At 10/18/2011 09:32 AM, Wen Congyang Write:
At 10/18/2011 01:04 AM, Eric Blake Write:
On 10/17/2011 11:03 AM, Eric Blake wrote:
On 10/17/2011 10:17 AM, Alex Jia wrote:
Eric,
It's latest libvirt upstream, current commit id is commit 0a71c79.
# rpm -q glibc
glibc-2.12-1.42.el6.x86_64
430 if (strchr(toescape, *cur))
CC libvirt_util_la-buf.lo
util/buf.c: In function 'virBufferEscape':
util/buf.c:430: warning: logical '' with non-zero constant will
always
evaluate as true [-Wlogical-op]
Something's screwy. There's no in that line, unless strchr() is a
macro based on your particular compilation flags. I'm suspecting that
this is a case of -D_FORTIFY_SOURCE going awry. I don't see how this
could possibly be a libvirt issue; more likely, it is an issue with the
choice of compiler flags you are using, coupled with an issue in the
system headers causing the preprocessor to expand strchr() into
something that tickles a spurious gcc warning.
Just to make sure, could you show 'make V=1' output, so we know exactly
which compiler flags are in effect?
I can reproduce this problem, here is 'make V=1' output:
=
make[3]: Entering directory `/root/rpmbuild/BUILD/libvirt-0.9.6/src'
/bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H
-I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../src/util
-I../include -DIN_LIBVIRT-I/usr/include/libxml2 -Wall -W -Wformat-y2k
-Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas
-Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align
-Wwrite-strings -Wlogical-op -Waggregate-return -Wstrict-prototypes
-Wold-style-definition -Wmissing-prototypes -Wmissing-declarations
-Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls
-Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var
-Wdisabled-optimization -Wbuiltin-macro-redefined -Wmudflap
-Wpacked-bitfield-compat -Wsync-nand -Wattributes -Wcoverage-mismatch
-Wmultichar -Wno-missing-field-initializers -Wno-sign-compare
-Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-all
--param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables
-fdiagnostics
-s
how-option -funit-at-a-time -Werror -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT libvirt_util_la-buf.lo -MD
-MP -MF .deps/libvirt_util_la-buf.Tpo -c -o libvirt_util_la-buf.lo `test -f
'util/buf.c' || echo './'`util/buf.c
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib
-I../gnulib/lib -I../include -I../src/util -I../include -DIN_LIBVIRT
-I/usr/include/libxml2 -Wall -W -Wformat-y2k -Wformat-security -Winit-self
-Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow
-Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op
-Waggregate-return -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand
-Wattributes -Wcoverage-mismatch -Wmultichar -Wno-missing-field-initializers
-Wno-sign-compare -Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time -Werro
r
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT libvirt_util_la-buf.lo -MD
-MP -MF .deps/libvirt_util_la-buf.Tpo -c util/buf.c -fPIC -DPIC -o
.libs/libvirt_util_la-buf.o
cc1: warnings being treated as errors
util/buf.c: In function 'virBufferEscape':
util/buf.c:430: error: logical '' with non-zero constant will always evaluate
as true [-Wlogical-op]
make[3]: *** [libvirt_util_la-buf.lo] Error 1
=
If I remove -O2, I can build util/buf.c.
I test strchr in a small program:
=
# cat test.c
#includestdio.h
#includestring.h
int main() {
char *s = abcdefg;
int chr = (int)'d';
char *d;
d = strchr(s, chr);
if (d)
printf(%s\n, d);
return 0;
}
# gcc -std=c99 -Wall -o test test.c -Wlogical-op -Werror
# gcc -std=c99 -O2 -Wall -o test test.c -Wlogical-op -Werror
cc1: warnings being treated as errors
test.c: In function ‘main’:
test.c:9: error: logical ‘’ with non-zero constant will always evaluate as
true
# gcc -std=c99 -Wall -o test test.c -Wlogical-op -Werror
# gcc -std=c99 -Wall -o test test.c -Werror
=
The building failed only when '-O2' and '-Wlogical-op' are specified at the
same time.
[libvirt] [PATCH] compile: Add a missing function 'pciDeviceListFind' to libvirt_private.syms
compile error: ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuPrepareHostdevPCIDevices': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:183: undefined reference to `pciDeviceListFind' /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:230: undefined reference to `pciDeviceListFind' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuGetActivePciHostDeviceList': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:102: undefined reference to `pciDeviceListFind' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuDomainReAttachHostdevDevices': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:370: undefined reference to `pciDeviceListFind' Signed-off-by: Xu He Jie x...@linux.vnet.ibm.com --- src/libvirt_private.syms |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e03e33..61f4dc4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -892,6 +892,7 @@ pciDeviceListFree; pciDeviceListGet; pciDeviceListNew; pciDeviceListSteal; +pciDeviceListFind; pciDeviceNetName; pciDeviceReAttachInit; pciDeviceSetManaged; -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 02/13] virbuf: improve testsuite reporting
On 09/30/2011 12:22 AM, Eric Blake wrote: I had some temporary test failures while working on virbuf improvements in later patches, with output that looked like: Expected [] Actual [] which is pretty hard to figure out. Adding an Offset designation made it much easier to find which particular '' was at the wrong indentation, to fix the right part of the code. * tests/testutils.c (virtTestDifference): Make it easier to diagnose test failures. I'd like to say I like this tip. --- tests/testutils.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 08db732..7eb40f0 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -359,7 +359,7 @@ int virtTestDifference(FILE *stream, } /* Show the trimmed differences */ -fprintf(stream, \nExpect [); +fprintf(stream, \nOffset %d\nExpect [, (int) (expectStart - expect)); if ((expectEnd - expectStart + 1) fwrite(expectStart, (expectEnd-expectStart+1), 1, stream) != 1) return -1; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] compile: Add a missing function 'pciDeviceListFind' to libvirt_private.syms
At 10/18/2011 10:46 AM, Xu He Jie Write: compile error: ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuPrepareHostdevPCIDevices': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:183: undefined reference to `pciDeviceListFind' /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:230: undefined reference to `pciDeviceListFind' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuGetActivePciHostDeviceList': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:102: undefined reference to `pciDeviceListFind' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuDomainReAttachHostdevDevices': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:370: undefined reference to `pciDeviceListFind' Signed-off-by: Xu He Jie x...@linux.vnet.ibm.com --- src/libvirt_private.syms |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e03e33..61f4dc4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -892,6 +892,7 @@ pciDeviceListFree; pciDeviceListGet; pciDeviceListNew; pciDeviceListSteal; +pciDeviceListFind; The function is sorted by alphanumeric. pciDeviceNetName; pciDeviceReAttachInit; pciDeviceSetManaged; Ack with the nit fixed. Thanks Wen Congyang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] compile: Add a missing function 'pciDeviceListFind' to libvirt_private.syms
于 2011年10月18日 11:12, Wen Congyang 写道: At 10/18/2011 10:46 AM, Xu He Jie Write: compile error: ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuPrepareHostdevPCIDevices': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:183: undefined reference to `pciDeviceListFind' /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:230: undefined reference to `pciDeviceListFind' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuGetActivePciHostDeviceList': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:102: undefined reference to `pciDeviceListFind' ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_hostdev.o): In function `qemuDomainReAttachHostdevDevices': /home/soulxu/data/work-code/libvirt/src/qemu/qemu_hostdev.c:370: undefined reference to `pciDeviceListFind' Signed-off-by: Xu He Jiex...@linux.vnet.ibm.com --- src/libvirt_private.syms |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e03e33..61f4dc4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -892,6 +892,7 @@ pciDeviceListFree; pciDeviceListGet; pciDeviceListNew; pciDeviceListSteal; +pciDeviceListFind; The function is sorted by alphanumeric. pciDeviceNetName; pciDeviceReAttachInit; pciDeviceSetManaged; Ack with the nit fixed. Thanks Wen Congyang Thanks Xu He Jie -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: Honor the original properties of PCI device when detaching
This patch fixes two problems:
1) The device will be reattached to host even if it's not
managed, as there is a pciDeviceSetManaged.
2) The device won't be reattached to host with original
driver properly. As it doesn't honor the device original
properties which are maintained by driver-activePciHostdevs.
---
src/qemu/qemu_hotplug.c | 17 +
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bfa524b..718f223 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1956,6 +1956,7 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver
*driver,
qemuDomainObjPrivatePtr priv = vm-privateData;
int i, ret;
pciDevice *pci;
+pciDevice *activePci;
for (i = 0 ; i vm-def-nhostdevs ; i++) {
if (vm-def-hostdevs[i]-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
@@ -2015,16 +2016,16 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver
*driver,
detach-source.subsys.u.pci.bus,
detach-source.subsys.u.pci.slot,
detach-source.subsys.u.pci.function);
-if (!pci)
-ret = -1;
-else {
-pciDeviceSetManaged(pci, detach-managed);
-pciDeviceListDel(driver-activePciHostdevs, pci);
-if (pciResetDevice(pci, driver-activePciHostdevs, NULL) 0)
+if (pci) {
+activePci = pciDeviceListSteal(driver-activePciHostdevs, pci);
+if (pciResetDevice(activePci, driver-activePciHostdevs, NULL))
+qemuReattachPciDevice(activePci, driver);
+else
ret = -1;
-pciDeviceReAttachInit(pci);
-qemuReattachPciDevice(pci, driver);
pciFreeDevice(pci);
+pciFreeDevice(activePci);
+} else {
+ret = -1;
}
if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)
--
1.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 03/13] virbuf: more detailed error reporting
On 09/30/2011 12:22 AM, Eric Blake wrote: The next patch wants to add some sanity checking, which would be a different error than ENOMEM. Many existing callers blindly report OOM failure if virBuf reports an error, and this will be wrong in the (unlikely) case that they actually had a usage error instead; but since the most common error really is ENOMEM, I'm not going to fix all callers. Meanwhile, new discriminating callers can react differently depending on what failure happened. * src/util/buf.c (virBufferSetError): Add parameter. (virBufferGrow, virBufferVasprintf, virBufferEscapeString) (virBufferEscapeSexpr): Adjust callers. --- src/util/buf.c | 26 +- 1 files changed, 13 insertions(+), 13 deletions(-) I checked this patch, it seems everything is all right. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
