On Fri, Sep 7, 2012 at 3:26 AM, Jiri Denemark wrote:
> On Thu, Sep 06, 2012 at 17:50:25 -0600, Eric Blake wrote:
>> On 09/06/2012 05:31 PM, Doug Goldstein wrote:
>> > On Thu, Sep 6, 2012 at 10:09 AM, Jiri Denemark wrote:
>> >> If there is only one detail string for a particular event, we need to pu
>>
>> s/pu/put a/
>
> Oh my and I just pushed the series without fixing this typo :-/ At least it's
> not the first one and certainly not the last one either.
>
> Thanks for the reviews.
>
> Jirka
Ugh, sorry. I guess my brain has auto-correct built in because I
didn't see that. Eric has spotted numerous typos that others have
missed which is a good thing.
--
Doug Goldstein
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Recent spec file changes ensure that in distro situations, netcf
and libvirt will link against the same libnl in order to avoid
dumping core. But for every-day development, if you are F17 and
have the libnl3-devel headers available, libvirt was blindly
linking against libnl3 even though F17 netcf still links against
libnl1, making testing a self-built binary on F17 impossible.
By making configure a little bit smarter, we can avoid this
situation. I intentionally wrote the test so that we still favor
libnl-3 if netcf is not installed or if we couldn't use ldd
to determine which library netcf linked against.
* configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it.
---
Does this patch look safe enough to use? It was sufficient to
let me resume self-tests on my F17 box, where I had intentionally
installed libnl3-devel.
configure.ac | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 47a72b9..7528894 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2902,14 +2902,24 @@ LIBNL_LIBS=""
have_libnl=no
if test "$with_linux" = "yes"; then
-PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [
-have_libnl=yes
-AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0])
-AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is
available])
-PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0])
-LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS"
-LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS"
-], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
+# When linking with netcf, we must ensure that we pick the same version
+# of libnl that netcf picked. Prefer libnl-3 unless we can prove
+# netcf linked against libnl-1.
+ncftool=`which ncftool`
+case `(ldd "$ncftool") 2>&1` in
+*libnl.so.1*) ;;
+*)
+PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [
+have_libnl=yes
+AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0])
+AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is
available])
+PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0])
+LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS"
+LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS"
+], []) ;;
+esac
+if test "$have_libnl" = no; then
+PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [
have_libnl=yes
AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1],
[whether the netlink library is available])
@@ -2920,7 +2930,7 @@ if test "$with_linux" = "yes"; then
AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for
macvtap support])
fi
])
-])
+fi
fi
AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"])
--
1.7.11.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On RHEL 6.2, gcc 4.4.6 complains:
cc1: warning: command line option "-Wenum-compare" is valid for C++/ObjC++ but
not for C
which in turn breaks a -Werror build.
Meanwhile, in Fedora 17, gcc 4.7.0, -Wenum-compare has been enhanced
to also work on C, but at the same time, it is documented that -Wall
now implicitly includes -Wenum-compare.
Therefore, it is sufficient to remove explicit checks for this option,
avoiding the warning from older gcc while still getting the
compile-time safety from newer gcc.
* m4/virt-compile-warnings.m4 (-Wenum-compare): Omit explicit check.
---
Pushing under the build-breaker rule.
m4/virt-compile-warnings.m4 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index c3ff962..d1173eb 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -57,6 +57,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
dontwarn="$dontwarn -Wformat-nonliteral"
# Gnulib's stat-time.h violates this
dontwarn="$dontwarn -Waggregate-return"
+# gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall
+dontwarn="$dontwarn -Wenum-compare"
# Gnulib uses '#pragma GCC diagnostic push' to silence some
# warnings, but older gcc doesn't support this.
--
1.7.11.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/07/2012 09:13 AM, Daniel P. Berrange wrote:
> On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
>> When setting processor count for a domain using the API libvirt enforced
>> a maximum processor count that was determined using an IOCTL on
>> /dev/kvm. Unfortunately this value isn't representative enough and qemu
>> happily accepts and starts with values greater than the reported value.
>
> Really ? If KVM is accepting a greater vCPU count than it reports
> via /dev/kvm, then we should fix the latter
Why? We already allow oversubscription (you can run 3 guests with 1
vcpu each on a 2-cpu machine); we also allow pinning a 2-vcpu guest to 1
host cpu; so why can't we allow oversubscription from within a single
VM? Sure, performance will be lousier, but that doesn't mean the
request is invalid.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
The parent type for hostdev hybrid needs to be VIR_DOMAIN_DEVICE_NONE as the
device is passed into
the guest as a PCI Device.
In order to store the information of the NETDEV that is the parent of the
HOSTDEV in question we
use a new variable actualParent. This variable also helps during VF MAC
address, vlan and
virtportprofile configuration.
ActualParent = Parent in case of forward mode="hostdev"
---
src/conf/domain_conf.c |9 +++
src/conf/domain_conf.h |1 +
src/qemu/qemu_hostdev.c | 152 +--
src/qemu/qemu_hotplug.c |2 +-
4 files changed, 118 insertions(+), 46 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c59ea00..52c00db 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4587,6 +4587,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
hostdev->parent.type = VIR_DOMAIN_DEVICE_NET;
hostdev->parent.data.net = parent;
+hostdev->actualParent.type = VIR_DOMAIN_DEVICE_NET;
+hostdev->actualParent.data.net = parent;
hostdev->info = &parent->info;
/* The helper function expects type to already be found and
* passed in as a string, since it is in a different place in
@@ -4607,6 +4609,9 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
virDomainHostdevDefPtr hostdev = &actual->data.hostdev.def;
hostdev->parent.type = VIR_DOMAIN_DEVICE_NONE;
+hostdev->actualParent.type = VIR_DOMAIN_DEVICE_NET;
+hostdev->actualParent.data.net = parent;
+
if (VIR_ALLOC(hostdev->info) < 0) {
virReportOOMError();
goto error;
@@ -4990,6 +4995,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
hostdev = &def->data.hostdev.def;
hostdev->parent.type = VIR_DOMAIN_DEVICE_NET;
hostdev->parent.data.net = def;
+hostdev->actualParent.type = VIR_DOMAIN_DEVICE_NET;
+hostdev->actualParent.data.net = def;
hostdev->info = &def->info;
/* The helper function expects type to already be found and
* passed in as a string, since it is in a different place in
@@ -5011,6 +5018,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
case VIR_DOMAIN_NET_TYPE_HOSTDEV_HYBRID:
hostdev = &def->data.hostdev.def;
hostdev->parent.type = VIR_DOMAIN_DEVICE_NONE;
+hostdev->actualParent.type = VIR_DOMAIN_DEVICE_NET;
+hostdev->actualParent.data.net = def;
if (VIR_ALLOC(hostdev->info) < 0) {
virReportOOMError();
goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 171dd70..adbb777 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -377,6 +377,7 @@ struct _virDomainHostdevSubsys {
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
virDomainDeviceDef parent; /* higher level Def containing this */
+virDomainDeviceDef actualParent; /*used only in the case of hybrid
hostdev*/
int mode; /* enum virDomainHostdevMode */
unsigned int managed : 1;
unsigned int ephemeral : 1;
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 46c84b5..a060a7e 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -320,43 +320,87 @@ qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr
hostdev,
if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0)
return ret;
-vlan = virDomainNetGetActualVlan(hostdev->parent.data.net);
-virtPort = virDomainNetGetActualVirtPortProfile(
- hostdev->parent.data.net);
-if (virtPort) {
-if (vlan) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("direct setting of the vlan tag is not allowed "
- "for hostdev devices using %s mode"),
- virNetDevVPortTypeToString(virtPort->virtPortType));
-goto cleanup;
-}
-ret = qemuDomainHostdevNetConfigVirtPortProfile(linkdev, vf,
-virtPort, &hostdev->parent.data.net->mac, uuid,
-port_profile_associate);
-} else {
-/* Set only mac and vlan */
-if (vlan) {
-if (vlan->nTags != 1 || vlan->trunk) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("vlan trunking is not supported "
- "by SR-IOV network devices"));
+if (hostdev->parent.data.net) {
+vlan = virDomainNetGetActualVlan(hostdev->parent.data.net);
+virtPort =
virDomainNetGetActualVirtPortProfile(hostdev->parent.data.net);
+if (virtPort) {
+if (vlan) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("direct setting of the vlan tag is not
allowed "
+ "for hostdev devices using %s mode"),
+
This patch introduces the new forward mode='hostdev-hybrid' along with
attribute managed
Includes updates to the network RNG and new xml parser/formatter code.
---
docs/formatnetwork.html.in| 46 +
docs/schemas/network.rng |1 +
src/conf/network_conf.c |9 +++--
src/conf/network_conf.h |1 +
tests/networkxml2xmlin/hostdev-hybrid-pf.xml |7
tests/networkxml2xmlin/hostdev-hybrid.xml | 10 +
tests/networkxml2xmlout/hostdev-hybrid-pf.xml |7
tests/networkxml2xmlout/hostdev-hybrid.xml| 10 +
tests/networkxml2xmltest.c|2 +
9 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 49206dd..5a5392c 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -259,6 +259,22 @@
with .
+ hostdev-hybrid
+
+Libvirt later than 0.10.0 also supports "intelligent
+passthrough" of VF in the hybrid mode. This is done by
+using the
+functionality. Similar to
+the device's MAC address is first optionally configured and
+the device is optionally associated with an 802.1Qbh capable
+switch using an optionally specified
+element (see the examples of virtualport given above for
+type='direct' network devices). The Vf is passed into the
+guest as a PCI device and at the same time a virtual interface
+with type='direct' mode='bridge' is created in the guest. This
+hybrid mode of intelligent passthrough makes Live migration
+possible.
+
As mentioned above, a element can
have multiple subelements, each
@@ -341,6 +357,36 @@
...
+Similarly hostdev-hybrid mode can be used to support PCI-
+passthrough of VF in hybrid mode as described above. The
+interface pool can be specified with a list of
+ elements, each of which has
+< type> (must always be 'pci',
+, ,
+, and
+attributes or the interface pool can also be defined using a
+single physical function subelement to
+call out the corresponding physical interface associated with
+multiple virtual interfaces (similar to passthrough mode).
+
+
+...
+
+
+
+
+
+...
+
+
+
+...
+
+
+
+...
+
+
Quality of service
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 4abfd91..1f2136a 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -88,6 +88,7 @@
private
vepa
hostdev
+ hostdev-hybrid
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 9d53d8e..443b060 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -50,7 +50,7 @@
VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
- "none", "nat", "route", "bridge", "private", "vepa",
"passthrough", "hostdev")
+ "none", "nat", "route", "bridge", "private", "vepa",
"passthrough", "hostdev", "hostdev-hybrid")
VIR_ENUM_DECL(virNetworkForwardHostdevDevice)
VIR_ENUM_IMPL(virNetworkForwardHostdevDevice,
@@ -1289,6 +1289,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
case VIR_NETWORK_FORWARD_HOSTDEV:
+case VIR_NETWORK_FORWARD_HOSTDEV_HYBRID:
if (def->bridge) {
virReportError(VIR_ERR_XML_ERROR,
_("bridge name not allowed in %s mode (network
'%s')"),
@@ -1590,7 +1591,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def,
unsigned int flags)
virBufferAddLit(&buf, "forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
+if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV ||
+def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV_HYBRID) {
if (def->managed == 1)
virBufferAddLit(&buf, " managed='yes'");
else
@@ -1608,7 +1610,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def,
unsigned int flags)
if (def->nForwardIfs &&
This patch introduces the new interface type='hostdev-hybrid' along with
attribute managed
Includes updates to the domain RNG and new xml parser/formatter code.
Also introduces a ephemeral tag for hybrid hostdevs.
The ephemeral tag for hybrid hostdevs will be useful for live migration
support at a later stage.
---
docs/formatdomain.html.in | 29 ++
docs/schemas/domaincommon.rng | 50 ++
src/conf/domain_conf.c | 96 +---
src/conf/domain_conf.h |2 +
src/uml/uml_conf.c |5 +
src/xenxs/xen_sxpr.c |1 +
.../qemuxml2argv-net-hostdevhybrid.args|8 ++
.../qemuxml2argv-net-hostdevhybrid.xml | 35 +++
tests/qemuxml2argvtest.c |2 +
.../qemuxml2xmlout-net-hostdevhybrid.xml | 40
tests/qemuxml2xmltest.c|1 +
11 files changed, 256 insertions(+), 13 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 503685f..70cf362 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2659,6 +2659,20 @@
guest instead of .
+
+ Libvirt later than 0.10.0 also supports "intelligent passthrough"
+ of VF in the hybrid mode. This is done by using the functionality. Similar to the device's MAC address is first optionally
+ configured and the device is optionally associated with an 802.1Qbh
+ capable switch using an optionally specified
+ element (see the examples of virtualport given above for
+ type='direct' network devices). The Vf is passed into the guest as
+ a PCI device and at the same time a virtual interface with
+ type='direct' mode='bridge' is created in the guest. This hybrid mode
+ of intelligent passthrough makes Live migration possible.
+
+
...
@@ -2674,6 +2688,21 @@
...
+
+ ...
+
+
+
+
+
+
+
+
+
+
+
+ ...
+
Multicast tunnel
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c2c6184..eedc255 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1677,6 +1677,56 @@
+
+
+hostdev-hybrid
+
+
+
+
+yes
+no
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+pci
+
+
+
+
+
+usb
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8952b69..d8ab40c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -293,7 +293,8 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
"bridge",
"internal",
"direct",
- "hostdev")
+ "hostdev",
+ "hostdev-hybrid")
VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST,
"default",
@@ -1020,6 +1021,9 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
virDomainHostdevDefClear(&def->data.hostdev.def);
break;
+case VIR_DOMAIN_NET_TYPE_HOSTDEV_HYBRID:
+virDomainHostdevDefClear(&def->data.hostdev.def);
+break;
default:
break;
}
@@ -1072,6 +1076,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
virDomainHostdevDefClear(&def->data.hostdev.def);
break;
+case VIR_DOMAIN_NET_TYPE_HOSTDEV_HYBRID:
+virDomainHostdevDefClear(&def->data.hostdev.def);
+break;
+
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_LAST:
break;
@@ -1563,8 +1571,10 @@ void virD
This patch series adds the support for interface-type="hostdev-hybrid" and
forward mode="hostdev-hybrid".
The hostdev-hybrid mode makes migration possible along with PCI-passthrough.
I had posted a RFC on the hostdev-hybrid methodology earlier on the libvirt
mailing list.
The RFC can be found here:
https://www.redhat.com/archives/libvir-list/2012-February/msg00309.html
Shradha Shah (8):
RNG updates, new xml parser/formatter code for interface
type=hostdev-hybrid
RNG updates, new xml parser/formatter code for forward
mode=hostdev-hybrid
Hostdev-hybrid mode requires a direct linkdev and direct mode.
ActualParent is used to store the information about the NETDEV.
network: support hostdev-hybrid in network driver
qemu: support netdevs from hostdev-hybrid networks
Using the Ephemeral Flag to prepare for Migration Support.
Migration support for hostdev-hybrid.
docs/formatdomain.html.in | 29
docs/formatnetwork.html.in | 46 ++
docs/schemas/domaincommon.rng | 50 ++
docs/schemas/network.rng |1 +
include/libvirt/libvirt.h.in |1 +
src/conf/domain_conf.c | 167 +---
src/conf/domain_conf.h |8 +
src/conf/network_conf.c|9 +-
src/conf/network_conf.h|1 +
src/libvirt_private.syms |1 +
src/network/bridge_driver.c| 123 +--
src/qemu/qemu_command.c| 61 +++
src/qemu/qemu_domain.c |6 +-
src/qemu/qemu_domain.h |3 +-
src/qemu/qemu_driver.c |6 +-
src/qemu/qemu_hostdev.c| 158 +--
src/qemu/qemu_hotplug.c| 26 +++-
src/qemu/qemu_migration.c | 106 -
src/qemu/qemu_process.c|3 +-
src/uml/uml_conf.c |5 +
src/util/pci.c |2 +-
src/util/pci.h |2 +
src/util/virnetdev.c | 40 +
src/util/virnetdev.h |6 +
src/xenxs/xen_sxpr.c |1 +
tests/networkxml2xmlin/hostdev-hybrid-pf.xml |7 +
tests/networkxml2xmlin/hostdev-hybrid.xml | 10 ++
tests/networkxml2xmlout/hostdev-hybrid-pf.xml |7 +
tests/networkxml2xmlout/hostdev-hybrid.xml | 10 ++
tests/networkxml2xmltest.c |2 +
.../qemuxml2argv-net-hostdevhybrid.args|8 +
.../qemuxml2argv-net-hostdevhybrid.xml | 35
tests/qemuxml2argvtest.c |2 +
.../qemuxml2xmlout-net-hostdevhybrid.xml | 40 +
tests/qemuxml2xmltest.c|1 +
35 files changed, 884 insertions(+), 99 deletions(-)
create mode 100644 tests/networkxml2xmlin/hostdev-hybrid-pf.xml
create mode 100644 tests/networkxml2xmlin/hostdev-hybrid.xml
create mode 100644 tests/networkxml2xmlout/hostdev-hybrid-pf.xml
create mode 100644 tests/networkxml2xmlout/hostdev-hybrid.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdevhybrid.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdevhybrid.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-net-hostdevhybrid.xml
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/06/2012 04:52 PM, Eric Blake wrote:
> On 09/06/2012 07:43 AM, Daniel P. Berrange wrote:
>
>>> ===
>>> OPTION 3) Again, a single API, but with an added "nodespec" arg which would
>>> be used to describe the parent node you of the node you want added/updated
>>> to:
>>>
>>> 3) Can you think of a situation where this wouldn't work?
>> In a libvirt API where you don't use XML directly.
>>
>>
>> The thing I like about option 3 is that you are just passing in little
>> snippets of XML to either add/remove/modify. But we need to figure out
>> a better way todo this, without using XPath markers IMHO.
> Option 4
>
> Instead of an arbitrary XPath marker, can we come up with an enum of
> possible change sites? That would mean the user doesn't have quite as
> much flexibility (passing an int instead of string), but we are still
> extensible as new changeable sites make sense (add another enum
> constant). There's still some disambiguation needed, but I suppose you
> could obtain that by passing (a subset of) the original XML sufficient
> to provide the context necessary.
Yeah, this is more or less what we discussed separately on Wednesday,
and seems to fit with both Dan and DV's ideas.
>
> For example,
>
> /* full text of element to add to , embedded in
> * enough partial context from the original xml as to find the
> * right to add it to */
> virNetworkUpdate(VIR_NETWORK_UPDATE_DHCP,
> ""
> " "
> ""
> " " name='X.example.com' ip='10.24.75.10'/>"
> ""
> " "
> "",
I wouldn't include the whole hierarchy, though - just the
element. (instead of specifying adress='10.24.75.1' to figure out which
ip address, we could use DV's suggestion of including a simple "index"
argument). This may be slightly less flexible, but removes the need to
deal with an XML hierarchy, and works for current needs.
So the new API would be this:
int virNetworkUpdate(virNetworkPtr network,
unsigned int section,
size_t index,
char *xml,
unsigned int flags);
section would be one of the following:
typedef enum virNetworkUpdateSection {
VIR_NETWORK_SECTION_BRIDGE= 0,
VIR_NETWORK_SECTION_DOMAIN= 1,
VIR_NETWORK_SECTION_IP= 2,
VIR_NETWORK_SECTION_IP_DHCP_HOST = 3,
VIR_NETWORK_SECTION_IP_DHCP_RANGE = 4,
VIR_NETWORK_SECTION_FORWARD = 5,
VIR_NETWORK_SECTION_FORWARD_INTERFACE = 6,
VIR_NETWORK_SECTION_FORWARD_PF= 7,
VIR_NETWORK_SECTION_PORTGROUP = 8,
VIR_NETWORK_SECTION_DNS_HOST = 9,
VIR_NETWORK_SECTION_DNS_TXT = 10,
VIR_NETWORK_SECTION_DNS_SRV = 11,
};
In each case, the xml would need to be a full element named by the final
bit of the enum (so a bit of redundancy, but I think necessary for
consistency), and generally (with the exception of portgroup and dns
host) would not have any subelements (this both makes it simpler to
understand/translate into libvirt-glib, and limits the impact); for example:
virNetworkUpdate(net, VIR_NETWORK_SECTION_BRIDGE, 0
"",
VIR_NETWORK_UPDATE_LIVE|VIR_NETWORK_UPDATE_CONFIG);
(this would modify the existing element (presumably to turn off
stp); since UPDATE_ADD wasn't specified, if there was no bridge element
already present, this would fail). Note the unused "index" of 0.
virNetworkUpdate(net, VIR_NETWORK_SECTION_IP_DHCP_HOST, 0,
""
VIR_NETWORK_UPDATE_ADD | VIR_NETWORK_UPDATE_LIVE
| VIR_NETWORK_UPDATE_CONFIG);
(adds a new host. If any of the attributes match an existing host, it
would fail).
The index is still 0 - it could be changed to "1" if we wanted to change
a host entry in the dhcp of the network's 2nd ip address. (Should index
instead be a const char *, allowing for character indexes, like
"10.24.75.1"? Currently it's only there to allow for future expansion;
maybe a single integer doesn't allow for enough expansion, but a char*
would...) (or should I drop the whole idea of the index, and just
include it in a separate xml element at the beginning of the xml arg
when it becomes necessary?). It's important to notice that the index
isn't within the dhcp's host array, but within the parent IPs. This does
point out a possible deficiency with the idea of having a single index -
in the future there may be a case where multiple indexes are required.
Deleting a host would be done like this:
virNetworkUpdate(net, VIR_NETWORK_SECTION_IP_DHCP_HOST, 0,
/* just enough to match existing entry */
"",
VIR_NETWORK_UPDATE_DELETE | VIR_NETWORK_UPDATE_LIVE
| VIR_NETWORK_UPDATE_CON
On 120905 14:13:52, Dmitry Guryanov wrote:
> The goal of this patch series is to get rid of cached domains
> list.
> Someone can create several connections to libvirt or change
> something (start VM or change parameters) using native tools.
> In such case first connection will not get any info about those
> changes.
> There is no way to check, if the cache valid, so remove it and
> always query needed data using prlctl command.
>
Hello,
Please, disregard this patch series. Driver works too slow with it,
especially with virt-manager.
We decided to add special mode to prlctl, when it will listen for
events and print them to stdout in json format. Then we can keep
one prlctl process in such mode per libvirt connection and update
cached list of domains, when some events occur.
>
> Dmitry Guryanov (6):
> parallels: return a new list of domains from parallelsLoadDomains
> parallels: rename uuidstr to __uuidstr in macro
> parallels: return up-to-date info about domains
> parallels: don't track domain state in global domains list
> parallels: don't use stored domains list in parallelsDomainDefineXML
> parallels: remove domains list from _parallelsConn structure
>
> src/parallels/parallels_driver.c | 423
> ++
> src/parallels/parallels_utils.h |1 -
> 2 files changed, 195 insertions(+), 229 deletions(-)
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
> When setting processor count for a domain using the API libvirt enforced
> a maximum processor count that was determined using an IOCTL on
> /dev/kvm. Unfortunately this value isn't representative enough and qemu
> happily accepts and starts with values greater than the reported value.
Really ? If KVM is accepting a greater vCPU count than it reports
via /dev/kvm, then we should fix the latter
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07.09.2012 16:05, Eric Blake wrote:
> On 09/07/2012 06:51 AM, Peter Krempa wrote:
>> When setting processor count for a domain using the API libvirt enforced
>> a maximum processor count that was determined using an IOCTL on
>> /dev/kvm. Unfortunately this value isn't representative enough and qemu
>> happily accepts and starts with values greater than the reported value.
>
> But isn't there still _some_ reasonable limit that we should be
> checking? That is, although qemu will let me run a guest with 3 vcpus
> on my 2-cpu laptop, I'm sure that even qemu will reject an attempt to
> run 100 vcpus - how do we know what the real limit is?
>
> Also, I'm a bit worried that we may have other places in our code that
> might need fixing if vcpus > max pcpus, but I guess we'll discover those
> as we go along.
>
> As to the patch itself, the code looks fine; and since it only relaxes
> constraints, I think it is safe to apply; I'm just worried that we are
> relaxing too far, so you might want to wait for a second opinion or
> research further into the max limit enforced by qemu.
>
I am comfortable with taking this in. The VCPU count comes from user. It
is different from 'being secure by default' patch I've committed earlier
- setting RSS limit for qemu instance; I mean - the difference is qemu
can start to leak without any user interference which can lead to host
system trashing. However, if users wants to shoot themselves into the
leg and start million VCPU domain on a singlecore - well, that's their
own .
Michal
>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/07/2012 07:28 AM, Peter Krempa wrote:
> On 09/04/12 17:32, Osier Yang wrote:
>> Simply returns the storage volume objects. No supported filter
>> flags.
>>
>> include/libvirt/libvirt.h.in: Declare the API
>> python/generator.py: Skip the function for generating. virStoragePool.py
>> will be added in later patch.
>> src/driver.h: virDrvStoragePoolListVolumesFlags
>> src/libvirt.c: Implementation for the API.
>> src/libvirt_public.syms: Export the symbol to public
>> ---
>> include/libvirt/libvirt.h.in |3 ++
>> python/generator.py |1 +
>> src/driver.h |6 -
>> src/libvirt.c| 50
>> ++
>> src/libvirt_public.syms |1 +
>> 5 files changed, 60 insertions(+), 1 deletions(-)
>>
>
> Maybe there are some properties that we could use to filter storage
> volumes, but those can also be added later.
Agreed that additional filters can be added later. Off the top of my
head, I can think of several useful filters (and I'm pretty sure not all
of these filter idears are even exposed by current libvirt API, so we
would have to add other API before we could even filter on these bits):
whether the storage volume is encrypted, whether it is compressed
(possibly even by which compression method), whether the storage volume
is sparse or fully allocated, whether the storage volume has a backing
file or is standalone, whether the storage volume has internal
snapshots, current lock manager status (unlocked, shared lock,
read-write lock).
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/07/2012 06:51 AM, Peter Krempa wrote:
> When setting processor count for a domain using the API libvirt enforced
> a maximum processor count that was determined using an IOCTL on
> /dev/kvm. Unfortunately this value isn't representative enough and qemu
> happily accepts and starts with values greater than the reported value.
But isn't there still _some_ reasonable limit that we should be
checking? That is, although qemu will let me run a guest with 3 vcpus
on my 2-cpu laptop, I'm sure that even qemu will reject an attempt to
run 100 vcpus - how do we know what the real limit is?
Also, I'm a bit worried that we may have other places in our code that
might need fixing if vcpus > max pcpus, but I guess we'll discover those
as we go along.
As to the patch itself, the code looks fine; and since it only relaxes
constraints, I think it is safe to apply; I'm just worried that we are
relaxing too far, so you might want to wait for a second opinion or
research further into the max limit enforced by qemu.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/06/2012 01:54 PM, Gene Czarcinski wrote:
> On 09/06/2012 01:00 PM, Eric Blake wrote:
>> On 09/06/2012 10:08 AM, Gene Czarcinski wrote:
>>>This patch removed the "--filterwin2k" dnsmasq command line
>>>parameter which was unnecessary for domain specification,
>>>possibly blocked some usage, and was command line clutter.
>>>
>>>Gene Czarcinski
>>>
>> ACK and pushed. 'git am' didn't like the mail (it came through with
>> horrible whitespace corruption), so I had to hand-apply the entire patch
>> (manual tweak of bridge-driver.c, then a sed script to touch up the 9
>> test files), but I don't mind helping out on a first message while you
>> are still learning git. But expect a request for a repost if this
>> happens on future patches :)
>>
> I expect the the problem must be that I am using thunderbird as my
> regular email client. Now that I have more info about using "git
> send-email", maybe next time will be better.
Yes, Thunderbird does a very bad job of not mangling patches up. I've
tried everything I can find and haven't found anything that makes it better.
> I am sorry that this caused you more work. I thought about attaching
> the patch but the hacker document specifically requested inline and
> attached patches.
Heh. Yeah, I noticed that when making the small changes to HACKING
yesterday. Despite what it says, though, when I find the need to send a
patch via Thunderbird for some reason, I go against the recommendations
of HACKING and send it as an attached file instead of in the body of the
message. That produces an undamaged patch at the other end and, with
Thunderbird at least, it's possible to make in "inline" reply to the
patch by selecting the entire patch prior to hitting reply - this moves
the patch into the body of the message (with quotes).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/07/2012 08:06 AM, Daniel Veillard wrote:
On Fri, Sep 07, 2012 at 01:29:25PM +0200, Ján Tomko wrote:
On 09/07/12 05:25, Daniel Veillard wrote:
The problem is that libvirt and qemu releases are a priori not
tied, doing what you suggest would mean to try to guess the actual
qemu version used by the guest and then switch on or off, which would
somehow be at odd with the overall driver configuration.
This also raises the point of the semantic of -sandbox, the code
assumes that if it is not present then sandboxing is off, and if
it is present sandboxing is on, now what you say seems to imply that
sandboxing is on in 1.3 if not present. If right then we need to instead
do something like -sandbox=off to make sure we propagate the setting
assuming the qemu.conf explicitely states sandbox=0
So we are I think in a tristate configuration:
- sandbox=0 in qemu.conf
and we need to force it off if supported
- sandbox=1 in qemu.conf
and we need to force it on if supported
- commented out in qemu.conf
fallback to the qemu for that guest default
Yes, this tristate configuration makes sense to me.
Apparently currently -sandbox takes no arguments, any chance to
suport for -sandbox=off before 1.3 ? Because otherwise the global
settings of libvirt qemu driver will conflict with qemu default setting.
Daniel
-sandbox does require an argument, either on or off, so that tri-state
configuration is doable at the moment.
Ah, excellent !
I don't think having it on by default is a good idea at this time - I
had to add a few syscalls to the whitelist to get it working for me
before posting the patch, but somehow I managed to break it since.
Jan, What syscalls did you have to add?
We can try to keep commented out then, but we won't get much testing
then ...
We want all the testing we can get. At the same time, I think we'd also
like to have some more assurance that the whitelist is complete before
turning it on by default.
The QEMU 1.3 soft feature freeze is on Nov 1st. Should we let this bake
for a little bit with default off, and perhaps set a target date of Oct
1st to turn the default on?
--
Regards,
Corey
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
tools/Makefile.am: add make rules for manpages
tools/virsh.pod: add reference to keycodes manpages
.gitignore: ignore manpage files
---
.gitignore| 2 +-
tools/Makefile.am | 31 ---
tools/virsh.pod | 21 -
3 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/.gitignore b/.gitignore
index d998f0e..d0cd2bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -168,7 +168,7 @@
/tests/vmx2xmltest
/tests/xencapstest
/tests/xmconfigtest
-/tools/*.[18]
+/tools/*.[158]
/tools/libvirt-guests.init
/tools/libvirt-guests.service
/tools/virsh
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 0d7822d..370601c 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -33,10 +33,11 @@ EXTRA_DIST = \
virsh-network.c virsh-nodedev.c \
virsh-nwfilter.c virsh-pool.c \
virsh-secret.c virsh-snapshot.c \
- virsh-volume.c
-
+ virsh-volume.c \
+ keymap-gen.pl
+BUILT_SOURCES =
DISTCLEANFILES =
bin_SCRIPTS = virt-xml-validate virt-pki-validate
@@ -47,11 +48,29 @@ sbin_SCRIPTS = virt-sanlock-cleanup
DISTCLEANFILES += virt-sanlock-cleanup
endif
+LIBVIRT_KEYCODESET= \
+ libvirt-keycode-linux.5 \
+ libvirt-keycode-xt.5 \
+ libvirt-keycode-atset1.5 \
+ libvirt-keycode-atset2.5 \
+ libvirt-keycode-atset3.5 \
+ libvirt-keycode-os_x.5 \
+ libvirt-keycode-xt_kbd.5 \
+ libvirt-keycode-win32.5 \
+ libvirt-keycode-usb.5 \
+ libvirt-keycode-rfb.5
+
+BUILT_SOURCES += $(LIBVIRT_KEYCODESET)
+
dist_man1_MANS = \
virt-host-validate.1 \
virt-pki-validate.1 \
virt-xml-validate.1 \
virsh.1
+
+dist_man5_MANS = \
+ $(LIBVIRT_KEYCODESET)
+
if HAVE_SANLOCK
dist_man8_MANS = virt-sanlock-cleanup.8
endif
@@ -81,6 +100,13 @@ virt-sanlock-cleanup: virt-sanlock-cleanup.in Makefile
virt-sanlock-cleanup.8: virt-sanlock-cleanup.in
$(AM_V_GEN)$(POD2MAN) --name VIRT-SANLOCK-CLEANUP $< $(srcdir)/$@
+libvirt-keycode-%.in: keymap-gen.pl ../src/util/keymaps.csv
+ $(AM_V_GEN)$(PERL) keymap-gen.pl ../src/util/keymaps.csv
+
+libvirt-keycode-%.5: libvirt-keycode-%.in
+ $(AM_V_GEN)$(POD2MAN) --name VIRT-CODESET $< $@
+
+
virt_host_validate_SOURCES = \
virt-host-validate.c \
virt-host-validate-common.c virt-host-validate-common.h \
@@ -133,7 +159,6 @@ virsh_CFLAGS =
\
$(COVERAGE_CFLAGS) \
$(LIBXML_CFLAGS)\
$(READLINE_CFLAGS)
-BUILT_SOURCES =
if WITH_WIN_ICON
virsh_LDADD += virsh_win_icon.$(OBJEXT)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 68138e5..fc7ce67 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1252,53 +1252,55 @@ be chosen.
The numeric values are those defined by the Linux generic input
event subsystem. The symbolic names match the corresponding
-Linux key constant macro names.
+Linux key constant macro names. See L
=item B
The numeric values are those defined by the original XT keyboard
-controller. No symbolic names are provided
+controller. No symbolic names are provided. See L
=item B
The numeric values are those defined by the AT keyboard controller,
set 1 (aka XT compatible set). Extended keycoes from B
may differ from extended keycodes in the B codeset. No symbolic
-names are provided
+names are provided. See L
=item B
The numeric values are those defined by the AT keyboard controller,
-set 2. No symbolic names are provided
+set 2. No symbolic names are provided. See L
=item B
The numeric values are those defined by the AT keyboard controller,
-set 3 (aka PS/2 compatible set). No symbolic names are provided
+set 3 (aka PS/2 compatible set). No symbolic names are provided.
+See L
=item B
The numeric values are those defined by the OS-X keyboard input
subsystem. The symbolic names match the corresponding OS-X key
-constant macro names
+constant macro names. See L
=item B
The numeric values are those defined by the Linux KBD device.
These are a variant on the original XT codeset, but often with
different encoding for extended keycodes. No symbolic names are
-provided.
+provided. See L
=item B
The numeric values are those defined by the Win32 keyboard input
subsystem. The symbolic names match the corresponding Win32 key
-constant macro names
+constant macro names. See L
=item B
The numeric values are those defined by the USB HID specification
-for keyboard input. No symbolic names are provided
+for keyboard input. No symbolic names are provided.
+See L
=item B
@@ -1306,6 +1308,7 @@ The numeric values are those defined by the RFB extension
f
This patchset will try to generate manpages to describe valid keycodes.
It does this job via a helper perl script passing file src/util/keymaps.csv.
we have 1 man page per keycode set, that is, 11 manpages to be generated
in total. Each of them corresponds to codeset defined in virsh manpage.
libvirt-keycode-linux.5
libvirt-keycode-xt.5
libvirt-keycode-atset1.5
libvirt-keycode-atset2.5
libvirt-keycode-atset3.5
libvirt-keycode-os_x.5
libvirt-keycode-kbd.5
libvirt-keycode-win32.5
libvirt-keycode-usb.5
libvirt-keycode-rfb.5
I really think that the words in these manpages still be polished again.
If anyone could help, that would be appreciated.
Guannan Ren(3)
doc: Add a perl script to generate keycodes mapping
automake: generate libvirt keycodes manpages
doc: fix some typoes on virsh manpage
.gitignore | 2 +-
tools/Makefile.am | 31 +---
tools/keymap-gen.pl | 202 +
tools/virsh.pod | 27 +
4 files changed, 245 insertions(+), 17 deletions(-)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
---
tools/virsh.pod | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index fc7ce67..e7df72c 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -686,7 +686,7 @@ address of virtual interface (such as I or
I) will accept the MAC address printed by this command.
=item B I I I [I] [I<--shallow>]
-[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose]
+[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose>]
[{I<--pivot> | I<--finish>}] [I<--timeout> B] [I<--async>]]
Copy a disk backing image chain to I. By default, this command
@@ -727,7 +727,7 @@ I specifies fully-qualified path of the disk.
I specifies copying bandwidth limit in MiB/s.
=item B I I [I] [I]
-[I<--wait> [I<--verbose>] [I<--timeout> B] [I<--async]]
+[I<--wait> [I<--verbose>] [I<--timeout> B] [I<--async>]]
Populate a disk from its backing image chain. By default, this command
flattens the entire chain; but if I is specified, containing the
@@ -2825,8 +2825,6 @@ the result will also be converted back from QMP. If more
than one argument
is provided for I, they are concatenated with a space in between
before passing the single command to the monitor.
-=back
-
=item B I [I<--timeout> I | I<--async> |
I<--block>] I...
Send an arbitrary guest agent command I to domain I through
--
1.7.11.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/04/12 17:32, Osier Yang wrote:
Simply returns the storage volume objects. No supported filter
flags.
include/libvirt/libvirt.h.in: Declare the API
python/generator.py: Skip the function for generating. virStoragePool.py
will be added in later patch.
src/driver.h: virDrvStoragePoolListVolumesFlags
src/libvirt.c: Implementation for the API.
src/libvirt_public.syms: Export the symbol to public
---
include/libvirt/libvirt.h.in |3 ++
python/generator.py |1 +
src/driver.h |6 -
src/libvirt.c| 50 ++
src/libvirt_public.syms |1 +
5 files changed, 60 insertions(+), 1 deletions(-)
Maybe there are some properties that we could use to filter storage
volumes, but those can also be added later. This patch looks OK so ACK
if nobody else objects.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/04/12 17:32, Osier Yang wrote:
src/test/test_driver.c: Implement poolListAllVolumes.
---
src/test/test_driver.c | 67
1 files changed, 67 insertions(+), 0 deletions(-)
Same issue with the version tag as in 3/6 but also fixed in the repo.
ACK
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
s/Implemente/implement in subject
On 09/04/12 17:32, Osier Yang wrote:
The RPC generator doesn't returning support list of object, this
patch do the work manually.
* daemon/remote.c:
Implemente the server side handler remoteDispatchStoragePoolListAllVolumes
* src/remote/remote_driver.c:
Add remote driver handler remoteStoragePoolListAllVolumes
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
---
daemon/remote.c | 58
src/remote/remote_driver.c | 66 ++
src/remote/remote_protocol.x | 14 -
src/remote_protocol-structs | 13
4 files changed, 150 insertions(+), 1 deletions(-)
Otherwise looks OK. ACK
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 6 Sep 2012 14:54:35 +0100, "Daniel P. Berrange"
wrote:
> > docs/formatdomain.html.in|2 +
> > src/conf/domain_conf.c |3 +-
> > src/conf/domain_conf.h |2 +-
>
> Opps, forgot to mention in previous reviews that you must
> update docs/schemas/domaincommon.rng to list the new attribute
>
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 5472267..88b7e87 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -144,7 +144,6 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
> >"ich9-ahci",
> >"no-acpi",
> >"fsdev-readonly",
> > -
> >"virtio-blk-pci.scsi", /* 80 */
> >"blk-sg-io",
> >"drive-copy-on-read",
>
> Bogus whitespace removal
>
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 4ca3047..3f78635 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -46,6 +46,7 @@
> > #include
> > #include
> > #include
> > +#include
>
> You don't use any functions from this header, so it can be removed.
>
> > @@ -2632,9 +2634,59 @@ error:
> > return NULL;
> > }
> >
> > +/*
> > + * Invokes the Proxy Helper with one of the socketpair as its parameter
> > + *
> > + */
> > +static int qemuInvokeProxyHelper(const char *emulator, int sock, const
> > char *path)
> > +{
> > +#define HELPER "virtfs-proxy-helper"
> > +int ret_val, status;
> > +virCommandPtr cmd;
> > +char *helper, *dname, *dp;
> > +
> > +dp = strdup(emulator);
> > +if (!dp) {
> > +virReportOOMError();
> > +return -1;
>
> This doesn't close 'sock' like other error paths do
I will fix these issues.
> Hmm, right here we are in the middle of constructing the command line
> args for QEMU. We should not be launching at processes at this point,
> not least since we could be running this from the test suite.
>
ie we should invoke during qemuProcessStart(), I will modify.
> Also you don't appear to have any code to tell the daemonized proxy
> to shutdown when QEMU is stopped ?
>
When QEMU terminates (socket closed), proxy helper also terminates
(reading from socket fails and it terminates).
> We really need to push the invocation of the proxy up into the code
> which actually launches QEMU, and merely have this part construct the
> command line arguments.
>
Ok, I will apply these changes and change next version
Thanks for the review
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
> Let users add either files or devices as disks to domains.
> ---
> libvirt-designer/libvirt-designer-domain.c | 244
>
> libvirt-designer/libvirt-designer-domain.h |7 +
> libvirt-designer/libvirt-designer.sym |3 +
> 3 files changed, 254 insertions(+), 0 deletions(-)
>
> diff --git a/libvirt-designer/libvirt-designer-domain.c
> b/libvirt-designer/libvirt-designer-domain.c
> index a8cabde..20611f2 100644
> --- a/libvirt-designer/libvirt-designer-domain.c
> +++ b/libvirt-designer/libvirt-designer-domain.c
[...]
> @@ -663,3 +670,240 @@ cleanup:
> g_object_unref(guest);
> return ret;
> }
> +
> +static GList *
> +gvir_designer_domain_get_supported_disk_bus_types(GVirDesignerDomain *design)
> +{
> +GVirDesignerDomainPrivate *priv = design->priv;
> +OsinfoDeviceList *dev_list;
> +GHashTable *bus_hash = g_hash_table_new(g_str_hash, g_str_equal);
> +GList *ret = NULL;
> +int i;
> +
> +dev_list = osinfo_os_get_devices_by_property(priv->os, "class", "block",
> TRUE);
> +
> +for (i = 0; i < osinfo_list_get_length(OSINFO_LIST(dev_list)); i++) {
> +OsinfoDevice *dev =
> OSINFO_DEVICE(osinfo_list_get_nth(OSINFO_LIST(dev_list), i));
> +const gchar *bus = osinfo_device_get_bus_type(dev);
> +
> +if (bus)
> +g_hash_table_add(bus_hash, g_strdup(bus));
> +}
> +
> +ret = g_hash_table_get_keys(bus_hash);
> +ret = g_list_copy(ret);
It's probably OK here...
> +
> +g_hash_table_destroy(bus_hash);
...and here, but...
> +return ret;
> +}
> +
[...]
> +static const gchar *
> +gvir_designer_domain_get_preferred_disk_bus_type(GVirDesignerDomain *design,
> + GError **error)
> +{
> +OsinfoDevice *dev;
> +OsinfoDeviceLink *dev_link =
> gvir_designer_domain_get_preferred_device(design,
> +
> "block",
> +
> error);
> +const gchar *ret = NULL;
> +
> +if (!dev_link)
> +return NULL;
> +
> +dev = osinfo_devicelink_get_target(dev_link);
> +ret = osinfo_device_get_bus_type(dev);
...for example here, shouldn't this be checked for the value passed to
the function? I know osinfo uses asserts for some of these, but that's
hard to say which of these should be checked and what's the right
procedure as lots of programs throw out these. I personally don't like
these " CRITICAL " lines at all.
But I'll get to that in another patch anyway ;)
Other than that this patch seems decent, even though I didn't do such a
thorough investigation as I do with other patches as this is pretty new
project.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hey,
On Fri, Sep 07, 2012 at 02:55:51PM +0200, Michal Privoznik wrote:
> On 07.09.2012 14:47, Daniel P. Berrange wrote:
> > On Fri, Sep 07, 2012 at 02:44:03PM +0200, Christophe Fergeau wrote:
> >> On Fri, Sep 07, 2012 at 01:24:35PM +0100, Daniel P. Berrange wrote:
> >>> A nice long detailed explanation. I agree that this scenario you
> >>> outline is plausible as an explanation for why Boxes sometimes
> >>> stops getting events from libvirtd.
> >>
> >> I've ran more tests in the mean time without this patch applied, but
> >> with the one below to add some debugging:
> >>
> >> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> >> index 43ecdcf..33d90fb 100644
> >> --- a/src/conf/domain_event.c
> >> +++ b/src/conf/domain_event.c
> >> @@ -1501,7 +1501,13 @@ virDomainEventStateRegisterID(virConnectPtr conn,
> >> int ret = -1;
> >>
> >> virDomainEventStateLock(state);
> >> +VIR_WARN("RegisterID");
>
> [1]
>
> >>
> >> +if ((state->callbacks->count == 0) && (state->timer == -1)) {
> >> +if (state->queue->count != 0) {
> >> +VIR_WARN("REG: queue's not empty: %d", state->queue->count);
>
> I understand the WARN level here but not in [1]. Isn't DEBUG just enough
> there?
Oh this was just a debug patch, I'm not proposing it for inclusion, hence
the WARN so that the logs I'm interested in jump to my face during the
debugging ;)
Christophe
pgpG78pY0GdrE.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07.09.2012 14:47, Daniel P. Berrange wrote:
> On Fri, Sep 07, 2012 at 02:44:03PM +0200, Christophe Fergeau wrote:
>> On Fri, Sep 07, 2012 at 01:24:35PM +0100, Daniel P. Berrange wrote:
>>> A nice long detailed explanation. I agree that this scenario you
>>> outline is plausible as an explanation for why Boxes sometimes
>>> stops getting events from libvirtd.
>>
>> I've ran more tests in the mean time without this patch applied, but
>> with the one below to add some debugging:
>>
>> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
>> index 43ecdcf..33d90fb 100644
>> --- a/src/conf/domain_event.c
>> +++ b/src/conf/domain_event.c
>> @@ -1501,7 +1501,13 @@ virDomainEventStateRegisterID(virConnectPtr conn,
>> int ret = -1;
>>
>> virDomainEventStateLock(state);
>> +VIR_WARN("RegisterID");
[1]
>>
>> +if ((state->callbacks->count == 0) && (state->timer == -1)) {
>> +if (state->queue->count != 0) {
>> +VIR_WARN("REG: queue's not empty: %d", state->queue->count);
I understand the WARN level here but not in [1]. Isn't DEBUG just enough
there?
>> +}
>> +}
>> if ((state->callbacks->count == 0) &&
>> (state->timer == -1) &&
>> (state->timer = virEventAddTimeout(-1,
>> @@ -1584,6 +1590,7 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
>> {
>> int ret;
>>
>> +VIR_WARN("DeregisterID");
ditto
>> virDomainEventStateLock(state);
>> if (state->isDispatching)
>> ret = virDomainEventCallbackListMarkDeleteID(conn,
>> @@ -1596,6 +1603,9 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
>> state->timer != -1) {
>> virEventRemoveTimeout(state->timer);
>> state->timer = -1;
>> +if (state->queue->count != 0) {
>> +VIR_WARN("DEREG: queue's not empty: %d", state->queue->count);
>> +}
>> }
>>
>> virDomainEventStateUnlock(state);
>>
>>
>> I've hit the event lost issue once, and right when this started happening,
>> the log was:
>> 2012-09-06 11:37:06.094+: 30498: warning :
>> virDomainEventStateDeregisterID:1593 : DeregisterID
>> 2012-09-06 11:37:06.094+: 30498: warning :
>> virDomainEventStateDeregisterID:1607 : DEREG: queue's not empty: 1
>> 2012-09-06 11:45:42.363+: 30502: warning :
>> virDomainEventStateRegisterID:1504 : RegisterID
>> 2012-09-06 11:45:42.363+: 30502: warning :
>> virDomainEventStateRegisterID:1508 : REG: queue's not empty: 1
>>
>> and after that, no events and these warnings kept happening with an
>> increasing number of queued events which is consistent with the hypothesis I
>> made
>> in this patch.
>
> Great, that's pretty encouraging then.
>
> Daniel
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
> as we need this DB later to find an OS or hypervisor and supported
> devices.
> ---
> libvirt-designer/Makefile.am |1 +
> libvirt-designer/libvirt-designer-domain.c |5 +++-
> libvirt-designer/libvirt-designer-internal.h | 30
> ++
> libvirt-designer/libvirt-designer-main.c | 17 +-
> 4 files changed, 51 insertions(+), 2 deletions(-)
> create mode 100644 libvirt-designer/libvirt-designer-internal.h
>
Even though I'm not an expert for this g{lib,object}/libosinfo, I'll go
ahead and try to have a look at these patches. This first one seems fine
to me, so in case it means anything, ACK for this one.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
When setting processor count for a domain using the API libvirt enforced
a maximum processor count that was determined using an IOCTL on
/dev/kvm. Unfortunately this value isn't representative enough and qemu
happily accepts and starts with values greater than the reported value.
This patch removes the check so that users are able to use full
potential of their big boxes also when setting processor counts with the
API not just in XML.
---
src/qemu/qemu_driver.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e8e00c..6be3e5f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3595,7 +3595,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
virDomainObjPtr vm;
virDomainDefPtr persistentDef;
const char * type;
-int max;
int ret = -1;
bool maximum;
@@ -3645,20 +3644,11 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
goto endjob;
}
-if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not determine max vcpus for the domain"));
-goto endjob;
-}
-
-if (!maximum && vm->def->maxvcpus < max) {
-max = vm->def->maxvcpus;
-}
-
-if (nvcpus > max) {
+if (!maximum && nvcpus > vm->def->maxvcpus) {
virReportError(VIR_ERR_INVALID_ARG,
_("requested vcpus is greater than max allowable"
- " vcpus for the domain: %d > %d"), nvcpus, max);
+ " vcpus for the domain: %d > %d"),
+ nvcpus, vm->def->maxvcpus);
goto endjob;
}
--
1.7.12
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Sep 07, 2012 at 02:44:03PM +0200, Christophe Fergeau wrote:
> On Fri, Sep 07, 2012 at 01:24:35PM +0100, Daniel P. Berrange wrote:
> > A nice long detailed explanation. I agree that this scenario you
> > outline is plausible as an explanation for why Boxes sometimes
> > stops getting events from libvirtd.
>
> I've ran more tests in the mean time without this patch applied, but
> with the one below to add some debugging:
>
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index 43ecdcf..33d90fb 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -1501,7 +1501,13 @@ virDomainEventStateRegisterID(virConnectPtr conn,
> int ret = -1;
>
> virDomainEventStateLock(state);
> +VIR_WARN("RegisterID");
>
> +if ((state->callbacks->count == 0) && (state->timer == -1)) {
> +if (state->queue->count != 0) {
> +VIR_WARN("REG: queue's not empty: %d", state->queue->count);
> +}
> +}
> if ((state->callbacks->count == 0) &&
> (state->timer == -1) &&
> (state->timer = virEventAddTimeout(-1,
> @@ -1584,6 +1590,7 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
> {
> int ret;
>
> +VIR_WARN("DeregisterID");
> virDomainEventStateLock(state);
> if (state->isDispatching)
> ret = virDomainEventCallbackListMarkDeleteID(conn,
> @@ -1596,6 +1603,9 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
> state->timer != -1) {
> virEventRemoveTimeout(state->timer);
> state->timer = -1;
> +if (state->queue->count != 0) {
> +VIR_WARN("DEREG: queue's not empty: %d", state->queue->count);
> +}
> }
>
> virDomainEventStateUnlock(state);
>
>
> I've hit the event lost issue once, and right when this started happening,
> the log was:
> 2012-09-06 11:37:06.094+: 30498: warning :
> virDomainEventStateDeregisterID:1593 : DeregisterID
> 2012-09-06 11:37:06.094+: 30498: warning :
> virDomainEventStateDeregisterID:1607 : DEREG: queue's not empty: 1
> 2012-09-06 11:45:42.363+: 30502: warning :
> virDomainEventStateRegisterID:1504 : RegisterID
> 2012-09-06 11:45:42.363+: 30502: warning :
> virDomainEventStateRegisterID:1508 : REG: queue's not empty: 1
>
> and after that, no events and these warnings kept happening with an
> increasing number of queued events which is consistent with the hypothesis I
> made
> in this patch.
Great, that's pretty encouraging then.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Sep 07, 2012 at 01:24:35PM +0100, Daniel P. Berrange wrote:
> A nice long detailed explanation. I agree that this scenario you
> outline is plausible as an explanation for why Boxes sometimes
> stops getting events from libvirtd.
I've ran more tests in the mean time without this patch applied, but
with the one below to add some debugging:
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 43ecdcf..33d90fb 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1501,7 +1501,13 @@ virDomainEventStateRegisterID(virConnectPtr conn,
int ret = -1;
virDomainEventStateLock(state);
+VIR_WARN("RegisterID");
+if ((state->callbacks->count == 0) && (state->timer == -1)) {
+if (state->queue->count != 0) {
+VIR_WARN("REG: queue's not empty: %d", state->queue->count);
+}
+}
if ((state->callbacks->count == 0) &&
(state->timer == -1) &&
(state->timer = virEventAddTimeout(-1,
@@ -1584,6 +1590,7 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
{
int ret;
+VIR_WARN("DeregisterID");
virDomainEventStateLock(state);
if (state->isDispatching)
ret = virDomainEventCallbackListMarkDeleteID(conn,
@@ -1596,6 +1603,9 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
state->timer != -1) {
virEventRemoveTimeout(state->timer);
state->timer = -1;
+if (state->queue->count != 0) {
+VIR_WARN("DEREG: queue's not empty: %d", state->queue->count);
+}
}
virDomainEventStateUnlock(state);
I've hit the event lost issue once, and right when this started happening,
the log was:
2012-09-06 11:37:06.094+: 30498: warning :
virDomainEventStateDeregisterID:1593 : DeregisterID
2012-09-06 11:37:06.094+: 30498: warning :
virDomainEventStateDeregisterID:1607 : DEREG: queue's not empty: 1
2012-09-06 11:45:42.363+: 30502: warning :
virDomainEventStateRegisterID:1504 : RegisterID
2012-09-06 11:45:42.363+: 30502: warning :
virDomainEventStateRegisterID:1508 : REG: queue's not empty: 1
and after that, no events and these warnings kept happening with an
increasing number of queued events which is consistent with the hypothesis I
made
in this patch.
Christophe
pgpXUfr3jOYGp.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Sep 06, 2012 at 03:06:41PM +0200, Christophe Fergeau wrote:
> GNOME Boxes sometimes stops getting domain events from libvirtd, even
> after restarting it. Further investigation in libvirtd shows that
> events are properly queued with virDomainEventStateQueue, but the
> timer virDomainEventTimer which flushes the events and sends them to
> the clients never gets called. Looking at the event queue in gdb
> shows that it's non-empty and that its size increases with each new
> events.
>
> virDomainEventTimer is set up in virDomainEventStateRegister[ID]
> when going from 0 client connecte to 1 client connected, but is
> initially disabled. The timer is removed in
> virDomainEventStateRegister[ID] when the last client is disconnected
> (going from 1 client connected to 0).
>
> This timer (which handles sending the events to the clients) is
> enabled in virDomainEventStateQueue when queueing an event on an
> empty queue (queue containing 0 events). It's disabled in
> virDomainEventStateFlush after flushing the queue (ie removing all
> the elements from it). This way, no extra work is done when the queue
> is empty, and when the next event comes up, the timer will get
> reenabled because the queue will go from 0 event to 1 event, which
> triggers enabling the timer.
>
> However, with this Boxes bug, we have a client connected (Boxes), a
> non-empty queue (there are events waiting to be sent), but a disabled
> timer, so something went wrong.
>
> When Boxes connects (it's the only client connecting to the libvirtd
> instance I used for debugging), the event timer is not set as expected
> (state->timer == -1 when virDomainEventStateRegisterID is called),
> but at the same time the event queue is not empty. In other words,
> we had no clients connected, but pending events. This also explains
> why the timer never gets enabled as this is only done when an event
> is queued on an empty queue.
>
> I think this can happen if an event gets queued using
> virDomainEventStateQueue and the client disconnection happens before
> the event timer virDomainEventTimer gets a chance to run and flush
> the event. In this situation, virDomainEventStateDeregister[ID] will
> get called with a non-empty event queue, the timer will be destroyed
> if this was the only client connected. Then, when other clients connect
> at a later time, they will never get notified about domain events as
> the event timer will never get enabled because the timer is only
> enabled if the event queue is empty when virDomainEventStateRegister[ID]
> gets called, which will is no longer the case.
A nice long detailed explanation. I agree that this scenario you
outline is plausible as an explanation for why Boxes sometimes
stops getting events from libvirtd. It also explains why we don't
see it with VDSM - since they're a long running service, not a
frequently stoppped/started desktop app, they're much less likely
to hit the race.
> To avoid this issue, this commit makes sure to remove all events from
> the event queue when the last client in unregistered. As there is
> no longer anyone interested in receiving these events, these events
> are stale so there is no need to keep them around. A client connecting
> later will have no interest in getting events that happened before it
> got connected.
Yep, makes sense.
> ---
> src/conf/domain_event.c | 24 +---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index 43ecdcf..7ab973b 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -525,13 +525,13 @@ void virDomainEventFree(virDomainEventPtr event)
> }
>
> /**
> - * virDomainEventQueueFree:
> + * virDomainEventQueueClear:
> * @queue: pointer to the queue
> *
> - * Free the memory in the queue. We process this like a list here
> + * Removes all elements from the queue
> */
> static void
> -virDomainEventQueueFree(virDomainEventQueuePtr queue)
> +virDomainEventQueueClear(virDomainEventQueuePtr queue)
> {
> int i;
> if (!queue)
> @@ -541,6 +541,22 @@ virDomainEventQueueFree(virDomainEventQueuePtr queue)
> virDomainEventFree(queue->events[i]);
> }
> VIR_FREE(queue->events);
> +queue->count = 0;
> +}
> +
> +/**
> + * virDomainEventQueueFree:
> + * @queue: pointer to the queue
> + *
> + * Free the memory in the queue. We process this like a list here
> + */
> +static void
> +virDomainEventQueueFree(virDomainEventQueuePtr queue)
> +{
> +if (!queue)
> +return;
> +
> +virDomainEventQueueClear(queue);
> VIR_FREE(queue);
> }
>
> @@ -1559,6 +1575,7 @@ virDomainEventStateDeregister(virConnectPtr conn,
> state->timer != -1) {
> virEventRemoveTimeout(state->timer);
> state->timer = -1;
> +virDomainEventQueueClear(state->queue);
> }
>
> virDomainEventStateUnlock(state);
> @@ -1596,6 +1613,7 @@ virDomainEventStateDeregisterID(virConnectP
On Fri, Sep 07, 2012 at 01:29:25PM +0200, Ján Tomko wrote:
> On 09/07/12 05:25, Daniel Veillard wrote:
> >
> > The problem is that libvirt and qemu releases are a priori not
> > tied, doing what you suggest would mean to try to guess the actual
> > qemu version used by the guest and then switch on or off, which would
> > somehow be at odd with the overall driver configuration.
> > This also raises the point of the semantic of -sandbox, the code
> > assumes that if it is not present then sandboxing is off, and if
> > it is present sandboxing is on, now what you say seems to imply that
> > sandboxing is on in 1.3 if not present. If right then we need to instead
> > do something like -sandbox=off to make sure we propagate the setting
> > assuming the qemu.conf explicitely states sandbox=0
> >
> > So we are I think in a tristate configuration:
> >- sandbox=0 in qemu.conf
> > and we need to force it off if supported
> >- sandbox=1 in qemu.conf
> > and we need to force it on if supported
> >- commented out in qemu.conf
> > fallback to the qemu for that guest default
> >
> > Apparently currently -sandbox takes no arguments, any chance to
> > suport for -sandbox=off before 1.3 ? Because otherwise the global
> > settings of libvirt qemu driver will conflict with qemu default setting.
> >
> > Daniel
> >
> -sandbox does require an argument, either on or off, so that tri-state
> configuration is doable at the moment.
Ah, excellent !
> I don't think having it on by default is a good idea at this time - I
> had to add a few syscalls to the whitelist to get it working for me
> before posting the patch, but somehow I managed to break it since.
We can try to keep commented out then, but we won't get much testing
then ...
> I'll look into those tests/qemuhelp*.
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/06/2012 08:54 PM, Bharata B Rao wrote:
Vijay - would really like to have your inputs here...
- While the transport-type for a volume is shown as tcp in "gluster
volume info", libgfapi forces me to use transport=socket to access the
same volume from QEMU. So does "socket" mean "tcp" really ? If so,
should I just switch over to using transport=tcp from QEMU ? If not,
can you explain a bit about the difference b/n socket and tcp
transport types ?
I suggest that we switch over to using transport=tcp. "socket" is a
generic abstraction that is used by various transport types - tcp, rdma
and unix. This needs to change in libfgapi as well and that should
happen shortly.
- Also apart from socket (or tcp ?), rdma and unix, are there any
other transport options that QEMU should care about ?
The ones you enumerate should be good enough.
- Are rdma and unix transport types operational at the moment ? If
not, do you see them being used in gluster any time in the future ?
The reason behind asking this is to check if we are spending effort in
defining semantics in QEMU for a transport type that is never going to
be used in gluster. Also I see that "gluster volume create" supports
tcp and rdma but doesn't list unix as an option.
Yes, both rdma and unix transport types are operational at the moment.
From a volume perspective, only tcp and rdma are valid types right now.
unix transport is used for communication between related gluster
processes on the same node. There could be cases where the need to talk
to glusterd on the localhost through "unix" transport type might arise.
Hence we can define semantics for all three types - tcp, rdma and unix.
Thanks,
Vijay
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/07/12 05:25, Daniel Veillard wrote:
>
> The problem is that libvirt and qemu releases are a priori not
> tied, doing what you suggest would mean to try to guess the actual
> qemu version used by the guest and then switch on or off, which would
> somehow be at odd with the overall driver configuration.
> This also raises the point of the semantic of -sandbox, the code
> assumes that if it is not present then sandboxing is off, and if
> it is present sandboxing is on, now what you say seems to imply that
> sandboxing is on in 1.3 if not present. If right then we need to instead
> do something like -sandbox=off to make sure we propagate the setting
> assuming the qemu.conf explicitely states sandbox=0
>
> So we are I think in a tristate configuration:
>- sandbox=0 in qemu.conf
> and we need to force it off if supported
>- sandbox=1 in qemu.conf
> and we need to force it on if supported
>- commented out in qemu.conf
> fallback to the qemu for that guest default
>
> Apparently currently -sandbox takes no arguments, any chance to
> suport for -sandbox=off before 1.3 ? Because otherwise the global
> settings of libvirt qemu driver will conflict with qemu default setting.
>
> Daniel
>
-sandbox does require an argument, either on or off, so that tri-state
configuration is doable at the moment.
I don't think having it on by default is a good idea at this time - I
had to add a few syscalls to the whitelist to get it working for me
before posting the patch, but somehow I managed to break it since.
I'll look into those tests/qemuhelp*.
Ján
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
在 2012-09-06四的 15:27 +0900，Kamezawa Hiroyuki写道：
> (2012/09/06 14:32), liguang wrote:
> > Hello, Eric & Daniel
> >
> > 在 2012-09-05三的 11:08 -0600，Eric Blake写道：
> >> On 09/05/2012 02:48 AM, Daniel P. Berrange wrote:
> >
> > I really don't like the general design of this patch, even
> > ignoring all the code bugs. I think this entire patch is
> > really just a solution in search of a problem. Offline migration
> > is already possible with existing libvirt APIs:
> >>
> >> I agree that the existing patches are making too many assumptions and
> >> not honoring flags correctly; but I'm still not sure why the user must
> >> decompose offline migration into a sequence of calls...
> >
> > yes, my original thought was to do all things together.
> >
> >>
> >
> > domsrc = virDomainLookupByName(connsrc, "someguest");
> > xml = virDomainGetXMLDesc(domsrc);
> > domdst virDomainDefine(conndst, xml);
> >
>
> Um, maybe you mean offline migration is just redefinition of domain at
> target side, but what about disk images the domain used without sharing
> files between source and target, do we have to take a look at this case?
> >>>
> >>> Which can also be done already
> >>>
> >>> virStorageVolDownload + virStorageVolUpload
> >>
> >> ...when a single virMigrate API could do the same decomposition as
> >> syntactic sugar, if the patch were cleaned up to actually obey flags.
> >> That is, why must virMigrate be a live-only operation, forcing
> >> virt-manager and all other wrappers to re-implement the same giant
> >> sequence of API calls for offline migration?
> >>
> >
> > so, libvirt may prefer APIs do one thing only?
> > maybe I have to just migrate the definition.
> >
>
> Can you try to move the definition in an atomic way ?
> copy to the dest + delete the original with preventing other ops to the
> target vm. I hope virsh migrate can support this "move" of definition
>
Ok, let me try to do delete the domain configuration before migration,
I thought it's the task of passing VIR_MIGRATE_UNDEFINE_SOURCE flag to
virMigrate* APIs, but, seems too late, it occurs until migration
confirm, almost at the end.
> Thanks,
> -Kame
>
>
--
liguanglig.f...@cn.fujitsu.com
FNST linux kernel team
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Sep 06, 2012 at 17:50:25 -0600, Eric Blake wrote:
> On 09/06/2012 05:31 PM, Doug Goldstein wrote:
> > On Thu, Sep 6, 2012 at 10:09 AM, Jiri Denemark wrote:
> >> If there is only one detail string for a particular event, we need to pu
>
> s/pu/put a/
Oh my and I just pushed the series without fixing this typo :-/ At least it's
not the first one and certainly not the last one either.
Thanks for the reviews.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I am pleased to announce that a new release of the libvirt-snmp package,
version 0.0.3, is now available from
ftp://libvirt.org/libvirt/snmp/
The new package contains several bug fixes and enhancements.
Thanks to all the people involved in contributing to this release.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Sep 06, 2012 at 11:17:58PM -0600, Eric Blake wrote:
> On 09/06/2012 04:13 AM, Hu Tao wrote:
> > In many places we store bitmap info in a chunk of data
> > (pointed to by a char *), and have redundant codes to
> > set/unset bits. This patch extends virBitmap, and convert
> > those codes to use virBitmap in subsequent patches.
> > ---
>
> >
> > struct _virBitmap {
> > -size_t size;
> > -unsigned long *map;
> > +size_t size;/* size in bits */
> > +size_t size2; /* size in LONGs */
>
> The name 'size2' isn't very descriptive. Maybe we should rename to
> s/size/max_bit/ and s/size2/map_len/ for easier reading?
Thanks for suggestion.
>
> > +unsigned long *map; /* bits are stored in little-endian format */
>
> This comment...
>
> > +/* Helper function. caller must ensure b < bitmap->size */
> > +static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b)
> > +{
> > +return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b));
>
> ...and this code disagree. This code is reading in machine-native
> format, not little-endian. And I much prefer operating in
> machine-native format. Which means when converting from char* to long,
> you'll have to properly pass things through endian conversion, rather
> than requiring the longs to be little-endian.
OK.
>
> > +char *virBitmapFormat(virBitmapPtr bitmap)
> > +{
> > +virBuffer buf = VIR_BUFFER_INITIALIZER;
> > +int first = -1;
> > +int start, cur;
> > +int ret;
> > +bool isset;
> > +
> > +if (!bitmap)
> > +return NULL;
> > +
> > +cur = 0;
> > +start = -1;
> > +while (cur < bitmap->size) {
> > +ret = virBitmapGetBit(bitmap, cur, &isset);
>
> I'm wondering if we should optimize this by using things like ffsl() and
> iterating a long at a time for longs that are 0 or -1, rather than
> blindly processing one bit at a time. Or even make this use the new
> virBitmapNextSetBit, and have that function be optimized a bit more.
Will improve it.
>
> > +if (ret != 0)
> > +goto error;
> > +else if (isset) {
>
> Style. Since the else branch used {}, the if branch must also use {}.
OK.
>
> > +if (start == -1)
> > +start = cur;
> > +} else if (start != -1) {
> > +if (!first)
> > +virBufferAddLit(&buf, ",");
> > +else
> > +first = 0;
> > +if (cur == start + 1)
> > +virBufferAsprintf(&buf, "%d", start);
> > +else
> > +virBufferAsprintf(&buf, "%d-%d", start, cur - 1);
> > +start = -1;
> > +}
> > +cur++;
> > +}
> > +
> > +if (start != -1) {
> > +if (!first)
> > +virBufferAddLit(&buf, ",");
> > +if (cur == start + 1)
> > +virBufferAsprintf(&buf, "%d", start);
> > +else
> > +virBufferAsprintf(&buf, "%d-%d", start, cur - 1);
> > +}
> > +
> > +if (virBufferError(&buf)) {
> > +error:
> > +virBufferFreeAndReset(&buf);
> > +return NULL;
> > +}
> > +
> > +return virBufferContentAndReset(&buf);
>
> Ouch. If the bitset is completely unset, then this returns NULL for
> both errors and success. You need to special-case a map that is
> completely unset to return strdup("") instead.
OK.
>
> > +
> > +#ifdef __BIG_ENDIAN__
> > +static unsigned long
> > +virSwapEndian(unsigned long l)
>
> Yuck. __BIG_ENDIAN__ vs. __LITTLE_ENDIAN__ is not guaranteed to exist.
> And even if you can rely on it, there's bound to be better ways to
> implement this instead of open-coding it ourselves (not to mention that
> by avoiding the #ifdefs, we avoid introducing bugs in the big-endian
> code that cannot be detected on little-endian machines). (Hmm, too bad
> gnulib doesn't guarantee be32toh and be64toh).
>
> > +/**
> > + * virBitmapAllocFromData:
> > + * @data: the data
> > + * @len: length of @data in bytes
> > + *
> > + * Allocate a bitmap from a chunk of data containing bits
> > + * information
> > + *
> > + * Returns a pointer to the allocated bitmap or NULL if
> > + * memory cannot be allocated.
> > + */
> > +virBitmapPtr virBitmapAllocFromData(void *data, int len)
> > +{
> > +virBitmapPtr bitmap;
> > +#ifdef __BIG_ENDIAN__
> > +int i;
> > +#endif
> > +
> > +bitmap = virBitmapAlloc(len * CHAR_BIT);
> > +if (!bitmap)
> > +return NULL;
> > +
> > +memcpy(bitmap->map, data, len);
>
> Instead of trying to memcpy() and then conditionally virSwapEndian(), I
> would just do it the manual way of reading one byte at a time for both
> endian types. Fewer #ifdefs, less ugly code.
OK.
>
> > +int virBitmapToData(virBitmapPtr bitmap, char **data, int *dataLen)
> > +{
> > +int len;
> > +#ifdef __BIG_ENDIAN__
> > +unsigned long *l;
> > +#endif
> > +
> > +len = bitmap->size2 * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT);
> > +
> > +if (VIR_ALLOC_N(*data, l
On Thu, Sep 06, 2012 at 11:23:49PM -0600, Eric Blake wrote:
> On 09/06/2012 11:17 PM, Eric Blake wrote:
> > On 09/06/2012 04:13 AM, Hu Tao wrote:
> >> In many places we store bitmap info in a chunk of data
> >> (pointed to by a char *), and have redundant codes to
> >> set/unset bits. This patch extends virBitmap, and convert
> >> those codes to use virBitmap in subsequent patches.
> >> ---
> >
> >>
> >> struct _virBitmap {
> >> -size_t size;
> >> -unsigned long *map;
> >> +size_t size;/* size in bits */
> >> +size_t size2; /* size in LONGs */
> >
> > The name 'size2' isn't very descriptive. Maybe we should rename to
> > s/size/max_bit/ and s/size2/map_len/ for easier reading?
>
> If you go with the rename idea, then I'd split this into two patches -
> one that introduces the new field and names and retrofits all existing
> functions, but adds no new code other than the use of the new field; and
> the second that only adds the new functions.
OK.
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hi,
Overview Part of JNA API describes as following:
1. Description1:
If the native method returns char* and actually allocates memory, a return type
of Pointer should be used to avoid leaking the memory. It is then up to you to
take the necessary steps to free the allocated memory.
2. Description2:
Declare the method as returning a Structure of the appropriate type, then
invoke Structure.toArray(int) to convert to an array of initialized structures
of the appropriate size. Note that your Structure class must have a no-args
constructor, and you are responsible for freeing the returned memory if
applicable in whatever way is appropriate for the called function.
And the example code shows as following:
// Original C code
struct Display* get_displays(int* pcount);
void free_displays(struct Display* displays);
// Equivalent JNA mapping
Display get_displays(IntByReference pcount);
void free_displays(Display[] displays);
...
IntByReference pcount = new IntByReference();
Display d = lib.get_displays(pcount);
Display[] displays = (Display[])d.toArray(pcount.getValue());
...
lib.free_displays(displays);
That's to say. All the memory allocated by native code must be freed explicitly
in JNA part. We must add some free memory methods to support the memory-freeing.
Any comments?
B.R.
Benjamin Wang
-Original Message-
From: Daniel Veillard [mailto:veill...@redhat.com]
Sent: 2012年8月20日 14:25
To: Benjamin Wang (gendwang)
Cc: st...@tvnet.hu; daniel.schwa...@dtnet.de
Subject: Re: Memory free in libvirt JNA
On Mon, Aug 20, 2012 at 05:15:45AM +, Benjamin Wang (gendwang) wrote:
> Hi Veillard,
> Thanks for your reply. I checked the current Libvirt-JNA
> implementation. I find that a method named "free" defined in Domain
> class which is used to free the domain object. If this is mandatory, that's
> to say, we should a lot of methods into the current Libvirt-jna
> implementation to free the memory which is allocated by libvirt API. Please
> correct me!
As far as I understat free() is aliased as finalize() on that object so the
java runtime will call free() automatically on garbage collection. I'm not a
java expert, check some Java litterature for more details about how this is
done and the cases where
free() might be better called directly.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/06/2012 04:24 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> When building RPMs the host kernel cannot be assumed to match
> the target OS kernel. Thus auto-detecting /selinux vs
> /sys/fs/selinux based on the host kernel can result in the
This line ...
> wrong choice (eg F18 builds on a RHEL6 host kernel)
> ---
> libvirt.spec.in | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 044b00f..fb35934 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1222,6 +1222,15 @@ of recent versions of Linux (and other OSes).
> %if 0%{?enable_autotools}
> autoreconf -if
> %endif
> +
> +%if %{with_selinux}
> +%if %{?fedora} >= 17 || %{?rhel} >= 7
> +%define with_selinux_mount --with-selinux-mount="/sys/fs/cgroup"
... and this line say something else, did you mean "/sys/fs/selinux" in
here by any chance as well? :)
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
