Re: [libvirt] [PATCH] Test for object identity when checking for None in Python

[Claudio Bley]

At Fri, 23 Aug 2013 11:25:35 +0800,
Guannan Ren wrote:
> 
> On 08/22/2013 09:56 PM, Claudio Bley wrote:
> > Consistently use "is" or "is not" to compare variables to None,
> > because doing so is preferrable, as per PEP 8
> > (http://www.python.org/dev/peps/pep-0008/#programming-recommendations):
> >
> >> Comparisons to singletons like None should always be done with is or
> >> is not, never the equality operators.
> > Signed-off-by: Claudio Bley 
> > ---
> > Purely mechanical change, using:
> >
> > find . -name '*.py' -exec sed -i -e 's,[ \t][ \t]*!=[ \t][ \t]*None, is not 
> > None,g' '{}' \+
> > find . -name '*.py' -exec sed -i -e 's,[ \t][ \t]*==[ \t][ \t]*None, is 
> > None,g' '{}' \+
> 
> This change makes sense.
> ACK

Thanks, pushed now.

Claudio
-- 
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:

Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

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

[libvirt] [PATCH]LXC doc: Add warns if net namespace not enabled

[Chen Hanxiao]

From: Chen Hanxiao 

If we don't enable network namespace, we could shutdown host
by executing command 'shutdown' inside container.
This patch will add some warnings in LXC docs and give some
advice to readers.

Signed-off-by: Chen Hanxiao 
---
 docs/drvlxc.html.in |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
index 640968f..8f3a36a 100644
--- a/docs/drvlxc.html.in
+++ b/docs/drvlxc.html.in
@@ -50,6 +50,13 @@ processes inside containers cannot be securely isolated from 
host
 process without the use of a mandatory access control technology
 such as SELinux or AppArmor.
 
+
+WARNING: If 'net' namespace not enabled for container,
+host OS could be shutdown by executing command like 'reboot'
+inside container.So make sure 'net' namespace was available and
+set the  feature in the XML, or configure virtual NICs.
+Then this issue could be circumvented.
+
 
 Default container setup
 
-- 
1.7.1

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

Re: [libvirt] configuring a disconnected

[Doug Goldstein]

On Tue, Jul 2, 2013 at 3:07 PM, Dan Kenigsberg  wrote:

> On Mon, Jun 24, 2013 at 11:22:16AM +0100, Daniel P. Berrange wrote:
> > On Sun, Jun 23, 2013 at 03:34:06PM +0300, Dan Kenigsberg wrote:
> > > On Fri, Jun 21, 2013 at 04:31:53AM -0500, Daniel P. Berrange wrote:
> > > > On Thu, Jun 20, 2013 at 02:42:31PM +0300, Dan Kenigsberg wrote:
> > > > > Hi List,
> > > > >
> > > > > Like most of us, I've bought my actual computer with an Ethernet
> > > > > interface card, that I can connect to a wall jack at will. It's
> quite
> > > > > easy to disconnect the Ethernet cable from the wall, as well.
> > > > >
> > > > > I would like to expose this behavior to virtual computers, too. Via
> > > > > libvirt, of course. That's useful, since an admin may need to
> disconnect
> > > > > a running VM from a network temporarily, without resorting to
> > > > > hot-unplugging its nic.
> > > >
> > > > In the bug you filed requested this feature, you said that you want
> > > > this so that you can make oVirt configure bridging behind libvirts
> > > > back with Quantum.
> > >
> > > This is not exact. Bug 878481 - define a disconnected 
> > > was opened with the VM-connected-to-no-bridge in mind.
> > >
> > > Since libvirt is lacking this feature, oVirt has introduced an ugly
> > > hack: a dummy bridge, to which a disconnected VM is moved to. The
> > > interface fo this VM had to be set with  to avoid
> > > inter-VM communication.
> > >
> > > Only then came the Quantum use case.
> >
> > Sigh. In the BZ, your justification only mentioned that this is
> > required for integration with Quantum, never that it was required
> > separately.
>
> Verifying this statement is left as an excercise to the
> reader of https://bugzilla.redhat.com/show_bug.cgi?id=878481#c0 .
>
> >
> > > > As explained in the bug, this is a really bad way
> > > > todo this & should not be required - OpenStack Nova demonstrates you
> > > > can do the right thing wrt Quantum using exisiting libvirt config.
> > >
> > > I'm not sure that this is THE right thing. At the momement, both
> > > quantum's linuxbridge plugin and nova's LinuxBridgeInterfaceDriver have
> > > an ensure_vlan_bridge() method.
> > >
> > >
> > > I'm no OpenStack expert, but I think that the nicest separation between
> > > nova's and quantum's domains is the tap device (obviously only for
> > > networks which use tap devices). Quantum should create the Linux bridge
> > > and its underlying connectivity (or even worse for ovs with security
> > > groups...), and Nova should connect a newly-created VM to it.
> Otherwise,
> > > Nova would have to reimplement just about any extension that is
> > > introduced to Quantum.
> >
> > While there are many, many ways to configure a physical network,
> > there are a small, finite number of ways that you can connect a
> > virtual machine to a physical network. Thus while there can be
> > many different quantum plugins, Nova only needs to know about a
> > handful of VM configuration rules.
> >
> > > In particular, I worry about the mapping of a network to a physical
> > > device. Quantum's linuxbridge does it according to a preconfigured
> > > cfg.CONF.LINUX_BRIDGE.physical_interface_mappings. Does Nova's vif
> > > driver support something like this? From where does it collect this
> > > information?
> > >
> > > Anyway, would you suggest oVirt to implement its own
> > > ensure_vlan_bridge()? Or use the vif driver code? Or that of
> linuxbridge
> > > quantum plugin?
> >
> > I'm not sure you're looking at current codebase. As of Grizzly the
> > quantum specific VIF plugin (and all other existing VIF plugins)
> > are deprecated in favour of LibvirtGenericVIFDriver. This defines
> > (currently) 4 different types of VIF configuration, linux bridge,
> > openvswitch, 802.qbg and 802.qbh. Each of these VIF types has a
> > set of metadata associated with it describing the configuration
> > requirements, which is used to configure the VM interface appropriately.
> > This clean separation isolates Nova from any knowledge of Quantum,
> > and vica-verca.
>
> I'm looking at the tip of master branch, both in
> https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L342
> and
>
> https://github.com/openstack/quantum/blob/master/quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py#L679
>
> My spefcific painpoint is that the vif driver calls
> linux_net.LinuxBridgeInterfaceDriver.ensure_vlan_bridge() with no
> regards of the configuration of the quantum agent.
>
> True, a specific configuration parameter of a specific agent of a
> quantum plugin is not of the business of Nova. But this suggests that
> connecting a bridge to an external NIC is not its business, either.
>
>
> >
> > > > So I'm not inclined to support this disconnected mode.
> > >
> > > Disconnected mode exists in actuality. It has valid use cases in the
> > > virtual world as well. I would like to discuss the domxml schema for
> > > representing it, and then, hopefully find the menpower to impl

[libvirt] [PATCH] Fix a memory leak in cmdSchedInfoUpdateOne

[hwbi2008]

From: hwbi 

The param needs to be virTypedParamsFree()'d in cmdSchedInfoUpdateOne().
---
 tools/virsh-domain.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b29f934..d704053 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4085,7 +4085,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl,
   int *nparams, int *maxparams,
   const char *field, const char *value)
 {
-virTypedParameterPtr param;
+virTypedParameterPtr param = NULL;
 int ret = -1;
 size_t i;
 
@@ -4109,6 +4109,7 @@ cmdSchedInfoUpdateOne(vshControl *ctl,
 vshError(ctl, _("invalid scheduler option: %s"), field);
 
  cleanup:
+virTypedParamsFree(param, *nparams);
 return ret;
 }
 
-- 
1.7.1

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

Re: [libvirt] [PATCH] Test for object identity when checking for None in Python

[Guannan Ren]

On 08/22/2013 09:56 PM, Claudio Bley wrote:

Consistently use "is" or "is not" to compare variables to None,
because doing so is preferrable, as per PEP 8
(http://www.python.org/dev/peps/pep-0008/#programming-recommendations):


Comparisons to singletons like None should always be done with is or
is not, never the equality operators.

Signed-off-by: Claudio Bley 
---
Purely mechanical change, using:

find . -name '*.py' -exec sed -i -e 's,[ \t][ \t]*!=[ \t][ \t]*None, is not 
None,g' '{}' \+
find . -name '*.py' -exec sed -i -e 's,[ \t][ \t]*==[ \t][ \t]*None, is None,g' 
'{}' \+


This change makes sense.
ACK

Guannan

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

Re: [libvirt] [PATCH] tests: skip schema validation tests if xmllint is missing

[Osier Yang]

On 23/08/13 04:08, Eric Blake wrote:

On IRC, someone complained that a system without xmllint installed
failed a number of tests.

* tests/schematestutils.sh: Probe for xmllint.

Signed-off-by: Eric Blake 
---
  tests/schematestutils.sh | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh
index e594f04..a0ff77e 100644
--- a/tests/schematestutils.sh
+++ b/tests/schematestutils.sh
@@ -1,5 +1,7 @@
  #!/bin/sh

+(xmllint --version) >/dev/null 2>&1 || skip_test_ 'Missing xmllint'
+
  check_schema () {

  DIRS=$1


ACK.

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

[libvirt] [PATCHv2 3/4] VMX: Some serial ports are not actually connected

[Doug Goldstein]

Sometimes a serial port might not be actually wired to a device when the
user does not have the VM powered on and we should not consider this a
fatal error.
---
 src/vmx/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index e7732e4..a13ef63 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2722,7 +2722,7 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, 
int port,
 
 /* vmx:fileName -> def:data.file.path */
 if (virVMXGetConfigString(conf, fileName_name, &fileName, false) < 0) {
-goto cleanup;
+goto ignore;
 }
 
 /* vmx:network.endPoint -> def:data.tcp.listen */
-- 
1.8.1.5

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

[libvirt] [PATCHv2 1/4] VMX: Add cdrom-raw dev type from VMWare Fusion

[Doug Goldstein]

According to VMWare's documentation 'cdrom-raw' is an acceptable value
for deviceType for a CD-ROM drive. The documentation states that the VMX
configuration for a CD-ROM deviceType is as follows:

ide|scsi(n):(n).deviceType = "cdrom-raw|atapi-cdrom|cdrom-image"

>From the documentation it appears the following is true:
- cdrom-image = Provides the ISO to the VM
- atapi-cdrom = Provides a NEC emulated ATAPI CD-ROM on top of the host
  CD-ROM
- cdrom-raw = Passthru for a host CD-ROM drive. Allows CD-R burning from
  within the guest.

A CD-ROM prior to this patch would always provide an 'atapi-cdrom' is
modeled as:
  



  

This patch allows an optional element as:

that would maintain existing behavior. While:

would provide a 'cdrom-raw' to the VM.
---
 docs/formatdomain.html.in  |  3 +-
 src/vmx/vmx.c  | 39 ++
 tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml |  1 +
 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx |  5 +++
 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml | 25 ++
 tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml|  1 +
 .../vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.vmx  |  6 
 .../vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml  | 25 ++
 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml|  1 +
 tests/vmx2xmltest.c|  2 ++
 .../xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.vmx | 13 
 .../xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.xml | 15 +
 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.vmx | 13 
 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.xml | 15 +
 .../xml2vmx-cdrom-scsi-atapi-device.vmx| 14 
 .../xml2vmx-cdrom-scsi-atapi-device.xml| 15 +
 .../xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.vmx  | 14 
 .../xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.xml  | 15 +
 tests/xml2vmxtest.c|  4 +++
 19 files changed, 218 insertions(+), 8 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-atapi-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-atapi-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9d12293..c52b2fa 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1829,7 +1829,8 @@
 supports a name of "tap", "tap2", "phy", or "file", with a
 type of "aio", while qemu only supports a name of "qemu",
 but multiple types including "raw", "bochs", "qcow2", and
-"qed".
+"qed". VMWare supports a name of "raw" or "atapi" for CD-ROMs,
+since 1.1.2.
   
   
 The optional cache attribute controls the
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 35afe26..ba549ab 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1064,6 +1064,14 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def,
 return 0;
 }
 
+/* Legitmate drivers are 'atapi' and 'raw' for CD-ROMs, so don't
+ * attempt to do legacy parsing on them.
+ */
+if (STRCASEEQ(disk->driverName, "atapi") ||
+STRCASEEQ(disk->driverName, "raw")) {
+return 0;
+}
+
 tmp = disk->driverName;
 
 for (; *tmp != '\0'; ++tmp) {
@@ -2174,12 +2182,13 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 goto cleanup;
 }
 } else if (virFileHasSuffix(fileName, ".iso") ||
-   STRCASEEQ(deviceType, "atapi-cdrom")) {
+   STRCASEEQ(deviceType, "atapi-cdrom") ||
+   STRCASEEQ(deviceType, "cdrom-raw")) {
 /*
  * This function was called in order to parse a harddisk device,
- * but .iso files and 'atapi-cdrom' devices are for CDROM devices
- * only. Just ignore it, another call to this function to parse a
- * CDROM device may handle it.
+ * but .iso files, 'atapi-cdrom', and 'cdrom-raw' devices are for
+ * CDROM devices only. Just ignore it, another call to this
+ * function to parse a CDROM device may handle it.
  

[libvirt] [PATCHv2 4/4] VMX: Add a VMWare Fusion 5 configuration for tests

[Doug Goldstein]

A user was having an issue with this specific VMWare Fusion config and
he gave me permission to add it as part of our test suite to further
expand our VMX test coverage. Unfortunately our VMX parser and
generator does not support many features contained within and just
silently ignores fields it does not understand so they had to
be removed out in the xml2vmx test. The original unmodified version
exists in the vmx2xml test.
---
 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++
 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 39 ++
 tests/vmx2xmltest.c|  2 +
 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx | 30 
 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 40 ++
 tests/xml2vmxtest.c|  2 +
 6 files changed, 201 insertions(+)
 create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml

diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx 
b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx
new file mode 100644
index 000..ef6af19
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx
@@ -0,0 +1,88 @@
+.encoding = "UTF-8"
+config.version = "8"
+virtualHW.version = "9"
+memsize = "3572"
+MemAllowAutoScaleDown = "FALSE"
+MemTrimRate = "-1"
+displayName = "ATTM_VM"
+guestOS = "winxppro"
+numvcpus = "2"
+sound.present = "TRUE"
+sound.filename = "-1"
+sound.autodetect = "TRUE"
+usb.present = "TRUE"
+ethernet0.present = "TRUE"
+ethernet0.addressType = "generated"
+ethernet0.connectionType = "bridged"
+ethernet1.present = "TRUE"
+ethernet1.addressType = "generated"
+ethernet1.connectionType = "bridged"
+scsi0:0.present = "TRUE"
+scsi0:0.fileName = "ATTM_VM.vmdk"
+pciBridge0.present = "TRUE"
+tools.upgrade.policy = "useGlobal"
+ehci.present = "TRUE"
+ide0:0.present = "TRUE"
+ide0:0.autodetect = "TRUE"
+ide0:0.filename = "auto detect"
+ide0:0.deviceType = "atapi-cdrom"
+scsi0.present = "TRUE"
+scsi0.virtualDev = "buslogic"
+buslogic.noDriver = "FALSE"
+extendedConfigFile = "ATTM_VM.vmxf"
+virtualHW.productCompatibility = "hosted"
+pciBridge4.present = "TRUE"
+pciBridge4.virtualDev = "pcieRootPort"
+pciBridge4.pciSlotNumber = "21"
+pciBridge4.functions = "8"
+pciBridge5.present = "TRUE"
+pciBridge5.virtualDev = "pcieRootPort"
+pciBridge5.pciSlotNumber = "22"
+pciBridge5.functions = "8"
+pciBridge6.present = "TRUE"
+pciBridge6.virtualDev = "pcieRootPort"
+pciBridge6.pciSlotNumber = "23"
+pciBridge6.functions = "8"
+pciBridge7.present = "TRUE"
+pciBridge7.virtualDev = "pcieRootPort"
+pciBridge7.pciSlotNumber = "24"
+pciBridge7.functions = "8"
+vmci0.present = "TRUE"
+hpet0.present = "TRUE"
+usb.vbluetooth.startConnected = "TRUE"
+mks.enable3d = "TRUE"
+ethernet0.linkStatePropagation.enable = "TRUE"
+ethernet1.linkStatePropagation.enable = "TRUE"
+ide0:0.startConnected = "FALSE"
+ethernet0.generatedAddress = "00:0c:29:3b:64:ea"
+ethernet1.generatedAddress = "00:0c:29:3b:64:f4"
+vmci0.id = "-952408854"
+tools.syncTime = "FALSE"
+uuid.location = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea"
+uuid.bios = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea"
+cleanShutdown = "TRUE"
+replay.supported = "FALSE"
+replay.filename = ""
+scsi0:0.redo = ""
+pciBridge0.pciSlotNumber = "17"
+scsi0.pciSlotNumber = "16"
+usb.pciSlotNumber = "32"
+ethernet0.pciSlotNumber = "33"
+ethernet1.pciSlotNumber = "34"
+sound.pciSlotNumber = "35"
+ehci.pciSlotNumber = "36"
+vmci0.pciSlotNumber = "37"
+usb:1.present = "TRUE"
+ethernet0.generatedAddressOffset = "0"
+ethernet1.generatedAddressOffset = "10"
+vmotion.checkpointFBSize = "134217728"
+usb:1.speed = "2"
+usb:1.deviceType = "hub"
+usb:1.port = "1"
+usb:1.parent = "-1"
+floppy0.startConnected = "FALSE"
+softPowerOff = "FALSE"
+usb:0.present = "TRUE"
+usb:0.deviceType = "hid"
+usb:0.port = "0"
+usb:0.parent = "-1"
diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml 
b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml
new file mode 100644
index 000..bed85d6
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml
@@ -0,0 +1,39 @@
+
+  ATTM_VM
+  564d7088-01a1-9832-e72b-6790c73b64ea
+  3657728
+  3657728
+  2
+  
+hvm
+  
+  
+  destroy
+  restart
+  destroy
+  
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+
+
+  
+  
+
+
+  
+  
+
+
+  
+
+  
+
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 5eb04e9..00bf21d 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -288,6 +288,8 @@ mymain(void)
 DO_TEST("ws-in-the-wild-1", "ws-in-the-wild-1");
 DO_TEST("ws-in-the-wild-2", "ws-in-the-wild-2");
 
+DO_TEST("fusion-in-the-wild-1", "fusion-in-the-wild-1");
+
 DO_TEST("an

[libvirt] [PATCHv2 2/4] VMX: Add support for 'auto detect' fileNames

[Doug Goldstein]

VMWare Fusion 5 can set the CD-ROM's device name to be 'auto detect' when
using the physical drive via 'cdrom-raw' device type. VMWare will then
connect to first available host CD-ROM to the virtual machine upon start
up according to VMWare documentation.

To better model this a CD-ROM that is marked as "auto detect" when in
the off state would be modeled as the following with this patch:
  


  

Note the tray state being marked as 'open', while the  element is
omitted.
---
 src/vmx/vmx.c  | 20 --
 .../vmx2xml-cdrom-ide-raw-auto-detect.vmx  |  5 +
 .../vmx2xml-cdrom-ide-raw-auto-detect.xml  | 24 ++
 .../vmx2xml-cdrom-scsi-raw-auto-detect.vmx |  6 ++
 .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 24 ++
 tests/vmx2xmltest.c|  2 ++
 .../xml2vmx-cdrom-ide-raw-auto-detect.vmx  | 14 +
 .../xml2vmx-cdrom-ide-raw-auto-detect.xml  | 14 +
 .../xml2vmx-cdrom-scsi-raw-auto-detect.vmx | 15 ++
 .../xml2vmx-cdrom-scsi-raw-auto-detect.xml | 14 +
 tests/xml2vmxtest.c|  2 ++
 11 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index ba549ab..e7732e4 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2227,9 +2227,14 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 } else if (STRCASEEQ(deviceType, "atapi-cdrom") ||
STRCASEEQ(deviceType, "cdrom-raw")) {
 (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
-(*def)->src = fileName;
 
-fileName = NULL;
+if (STRCASENEQ(fileName, "auto detect")) {
+(*def)->src = fileName;
+fileName = NULL;
+} else {
+(*def)->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
+(*def)->src = NULL;
+}
 
 if (STRCASEEQ(deviceType, "cdrom-raw")) {
 if (VIR_STRDUP((*def)->driverName, "raw") < 0) {
@@ -3546,6 +3551,11 @@ virVMXFormatCDROM(virVMXContext *ctx, 
virDomainDiskDefPtr def,
 VIR_FREE(fileName);
 }
 } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
+if (!def->src && def->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
+virBufferAsprintf(buffer, "%s%d:%d.autodetect = \"true\"\n",
+  entryPrefix, controllerOrBus, unit);
+}
+
 if (def->driverName && STRCASEEQ(def->driverName, "raw")) {
 virBufferAsprintf(buffer, "%s%d:%d.deviceType = \"cdrom-raw\"\n",
   entryPrefix, controllerOrBus, unit);
@@ -3557,6 +3567,12 @@ virVMXFormatCDROM(virVMXContext *ctx, 
virDomainDiskDefPtr def,
 if (def->src != NULL) {
 virBufferAsprintf(buffer, "%s%d:%d.fileName = \"%s\"\n",
   entryPrefix, controllerOrBus, unit, def->src);
+} else {
+if (def->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
+virBufferAsprintf(buffer, "%s%d:%d.fileName = "
+  "\"auto detect\"\n", entryPrefix,
+  controllerOrBus, unit);
+}
 }
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx 
b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx
new file mode 100644
index 000..b2c4caf
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx
@@ -0,0 +1,5 @@
+config.version = "8"
+virtualHW.version = "4"
+ide0:0.present = "true"
+ide0:0.deviceType = "cdrom-raw"
+ide0:0.fileName = "auto detect"
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml 
b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml
new file mode 100644
index 000..1e32be5
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml
@@ -0,0 +1,24 @@
+
+  ----
+  32768
+  32768
+  1
+  
+hvm
+  
+  
+  destroy
+  restart
+  destroy
+  
+
+  
+  
+  
+
+
+
+  
+
+  
+
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx 
b/tests/vmx2xmldata/

[libvirt] [PATCHv2 0/4] VMX: CD-ROM handling improvements

[Doug Goldstein]

A user came into #virt the other day and was trying to get libvirtd
to work with VMWare Fusion 5, which is basically the Mac OS X version of
VMWare Workstation. In helping him out I noticed a few limitations of our
VMX parser so I've added support through this patchset. However I came
across the fact that we only support 2 types of CD-ROMs instead of the 3
types that VMWare has lead to adding support for a  element to
CD-ROM drives. Additionally VMWare Fusion has a concept of auto detecting
your host CD-ROM drive at boot and we don't have a XML field for this so
I omitted a  element and added the tray to be opened.

Doug Goldstein (4):
  VMX: Add cdrom-raw dev type from VMWare Fusion
  VMX: Add support for 'auto detect' fileNames
  VMX: Some serial ports are not actually connected
  VMX: Add a VMWare Fusion 5 configuration for tests

 docs/formatdomain.html.in  |  3 +-
 src/vmx/vmx.c  | 61 ---
 tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml |  1 +
 .../vmx2xml-cdrom-ide-raw-auto-detect.vmx  |  5 ++
 .../vmx2xml-cdrom-ide-raw-auto-detect.xml  | 24 ++
 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx |  5 ++
 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml | 25 ++
 tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml|  1 +
 .../vmx2xml-cdrom-scsi-raw-auto-detect.vmx |  6 ++
 .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 24 ++
 .../vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.vmx  |  6 ++
 .../vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml  | 25 ++
 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml|  1 +
 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++
 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 39 ++
 tests/vmx2xmltest.c|  6 ++
 .../xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.vmx | 13 
 .../xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.xml | 15 
 .../xml2vmx-cdrom-ide-raw-auto-detect.vmx  | 14 
 .../xml2vmx-cdrom-ide-raw-auto-detect.xml  | 14 
 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.vmx | 13 
 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.xml | 15 
 .../xml2vmx-cdrom-scsi-atapi-device.vmx| 14 
 .../xml2vmx-cdrom-scsi-atapi-device.xml| 15 
 .../xml2vmx-cdrom-scsi-raw-auto-detect.vmx | 15 
 .../xml2vmx-cdrom-scsi-raw-auto-detect.xml | 14 
 .../xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.vmx  | 14 
 .../xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.xml  | 15 
 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx | 30 
 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 40 ++
 tests/xml2vmxtest.c|  8 ++
 31 files changed, 558 insertions(+), 11 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml
 create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-atapi-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-atapi-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-atapi-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.xml
 create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx
 create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml

-- 
1.8.1.5

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

Re: [libvirt] [PATCHv3 1/5] domifaddr: Implement the public API

[Eric Blake]

On 08/22/2013 04:18 PM, nehaljwani wrote:

[meta-review; I'm saving the technical review for later]

> Define a new API virDomainInterfacesAddresses, which returns

Your previous submission
https://www.redhat.com/archives/libvir-list/2013-August/msg01206.html
began with this line in the body (well, munged by the archives, but you
get the drift):

From: Nehal J Wani 

But this submission does not.  If I use 'git am' directly on your mail,
then 'git shortlog' would show two different groupings for patches from
you unless we also patch .mailmap to cover the difference in naming.
Furthermore, _this_ email claims to be from 'nehaljwani', while your
reply in this series,
https://www.redhat.com/archives/libvir-list/2013-August/msg01221.html,
claims to be from 'Nehal J. Wani'.  That's now three spellings for mail
purporting to be the same address.

While inconsistency in the use of 'J' vs. 'J.' in your initial is not
the end of the world, we definitely prefer legal names rather than
pseudonyms (such as your login name) in git authorship information.
Therefore, you ought to figure out how to configure 'git send-email' to
match the preferred spelling of your non-git mails.  After all, git only
adds a From: line to the top of a patch when the authorship is different
from the mail sender configuration, and while a From: line in the body
of an email generally means that someone else wrote the patch (fine),
seeing a From: line for your own submission is generally a sign of a
mis-configuration (embarrassing).

If you have a reason to resend this series, you should rebase it and use
'git commit --amend --author=' on each patch to adjust the
authorship information.  No need to resend just yet: wait for the
technical review. If I find nothing wrong in the technical review that
warrants a resend for other reasons, then I don't mind touching up
authorship this time around, since it is still one of your first
contributions.  But the bar gets higher the more you contribute :)

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] memory leak in snapshot and since at least 1.0.2?

[Serge Hallyn]

Quoting Eric Blake (ebl...@redhat.com):
> On 07/26/2013 10:09 AM, Serge Hallyn wrote:
> > Quoting Serge Hallyn (serge.hal...@ubuntu.com):
> >> Hi,
> >>
> >> https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1201938 documents
> >> a memory leak we're seeing in libvirt.  I've reproduced it in 1.0.2,
> >> 1.0.6, and an hourly snapshot from yesterday morning (which is built
> >> at https://launchpad.net/~serge-hallyn/+archive/libvirt-mav)
> >>
> >> To reproduce it, I define and start one domain and run virt-manager
> >> locally (just leaving it running in a vnc server).  The RSS as observed
> >> by top grows over time, starting at 10M and becoming 87M in about 12
> >> hours.
> >>
> >> When I stop the running vm, the memleak does seem to stop.  Presumably
> >> having >1 domain would speed up the memleak (explaining the original
> >> bug reporter's woes)
> > 
> > No, the memory leak did not stop altogether with the VM stopped.  Since
> > sending that email RSS has gone up by about 2M.
> 
> We recently found a memory leak bug in netcf that impacts debian-based
> builds but not Fedora-based builds.  Does your issue go away once you
> pick up that fix?
> https://lists.fedorahosted.org/pipermail/netcf-devel/2013-August/000844.html

Yup - well, the original reporter says there remains a very slow leak,
but I've not reproduced that one yet.  The netcf one was definately
responsible for the major leak.

thanks!

-serge

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

Re: [libvirt] memory leak in snapshot and since at least 1.0.2?

[Eric Blake]

On 07/26/2013 10:09 AM, Serge Hallyn wrote:
> Quoting Serge Hallyn (serge.hal...@ubuntu.com):
>> Hi,
>>
>> https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1201938 documents
>> a memory leak we're seeing in libvirt.  I've reproduced it in 1.0.2,
>> 1.0.6, and an hourly snapshot from yesterday morning (which is built
>> at https://launchpad.net/~serge-hallyn/+archive/libvirt-mav)
>>
>> To reproduce it, I define and start one domain and run virt-manager
>> locally (just leaving it running in a vnc server).  The RSS as observed
>> by top grows over time, starting at 10M and becoming 87M in about 12
>> hours.
>>
>> When I stop the running vm, the memleak does seem to stop.  Presumably
>> having >1 domain would speed up the memleak (explaining the original
>> bug reporter's woes)
> 
> No, the memory leak did not stop altogether with the VM stopped.  Since
> sending that email RSS has gone up by about 2M.

We recently found a memory leak bug in netcf that impacts debian-based
builds but not Fedora-based builds.  Does your issue go away once you
pick up that fix?
https://lists.fedorahosted.org/pipermail/netcf-devel/2013-August/000844.html

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3 3/5] domifaddr: Implement the API for qemu

[Nehal J. Wani]

Addition to the commit log:

tests/qemuagenttest.c
  * Add a test case for qemuAgentGetInterfaces




On Fri, Aug 23, 2013 at 3:48 AM, nehaljwani  wrote:

> By querying the qemu guest agent with the QMP command
> "guest-network-get-interfaces" and converting the received
> JSON output to structured objects.
>
> src/qemu/qemu_agent.h:
>   * Define qemuAgentGetInterfaces
>
> src/qemu/qemu_agent.c:
>   * Implement qemuAgentGetInterface
>
> src/qemu/qemu_driver.c:
>   * New function qemuDomainInterfacesAddresses
>
> src/remote_protocol-sructs:
>   * Define new structs
>
> ---
>  src/qemu/qemu_agent.c  | 157
> +
>  src/qemu/qemu_agent.h  |   3 +
>  src/qemu/qemu_driver.c |  55 +
>  tests/qemuagenttest.c  |  90 +++-
>  4 files changed, 304 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 2cd0ccc..015e752 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -844,6 +844,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
>  VIR_DEBUG("Attempt to send command while error is set %s",
>NULLSTR(mon->lastError.message));
>  virSetError(&mon->lastError);
> +printf("Upper\n");
>  return -1;
>  }
>
> @@ -862,6 +863,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
>  while (!mon->msg->finished) {
>  if ((then && virCondWaitUntil(&mon->notify, &mon->parent.lock,
> then) < 0) ||
>  (!then && virCondWait(&mon->notify, &mon->parent.lock) < 0)) {
> +
>  if (errno == ETIMEDOUT) {
>  virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
> _("Guest agent not available for now"));
> @@ -1319,6 +1321,161 @@ cleanup:
>  return ret;
>  }
>
> +
> +int
> +qemuAgentGetInterfaces(qemuAgentPtr mon,
> +   virDomainInterfacePtr **ifaces)
> +{
> +int ret = -1;
> +size_t i, j;
> +int size = -1;
> +virJSONValuePtr cmd = NULL;
> +virJSONValuePtr reply = NULL;
> +virJSONValuePtr ret_array = NULL;
> +int ifaces_count = 0;
> +virDomainInterfacePtr *ifaces_ret = NULL;
> +
> +if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces",
> NULL)))
> +   return -1;
> +
> +if (qemuAgentCommand(mon, cmd, &reply,
> VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
> +qemuAgentCheckError(cmd, reply) < 0) {
> +goto cleanup;
> +}
> +
> +if (!(ret_array = virJSONValueObjectGet(reply, "return"))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("qemu agent didn't provide 'return' field"));
> +goto cleanup;
> +}
> +
> +if ((size = virJSONValueArraySize(ret_array)) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("qemu agent didn't return an array of
> interfaces"));
> +goto cleanup;
> +}
> +
> +ifaces_count = (unsigned int) size;
> +
> +if (VIR_ALLOC_N(ifaces_ret, size) < 0)
> +goto cleanup;
> +
> +for (i = 0; i < size; i++) {
> +virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i);
> +virJSONValuePtr ip_addr_arr = NULL;
> +const char *name, *hwaddr;
> +int ip_addr_arr_size;
> +
> +if (VIR_ALLOC(ifaces_ret[i]) < 0)
> +goto cleanup;
> +
> +/* Shouldn't happen but doesn't hurt to check neither */
> +if (!tmp_iface) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("something has went really wrong"));
> +goto error;
> +}
> +
> +/* interface name is required to be presented */
> +name = virJSONValueObjectGetString(tmp_iface, "name");
> +if (!name) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("qemu agent didn't provide 'name' field"));
> +goto error;
> +}
> +
> +if (VIR_STRDUP(ifaces_ret[i]->name, name) < 0)
> +goto error;
> +
> +/* hwaddr might be omitted */
> +hwaddr = virJSONValueObjectGetString(tmp_iface,
> "hardware-address");
> +if (hwaddr && VIR_STRDUP(ifaces_ret[i]->hwaddr, hwaddr) < 0)
> +goto error;
> +
> +/* as well as IP address which - moreover -
> + * can be presented multiple times */
> +ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses");
> +if (!ip_addr_arr)
> +continue;
> +
> +if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0)
> +/* Mmm, empty 'ip-address'? */
> +continue;
> +
> +(*(ifaces_ret)[i]).naddrs = (unsigned int) ip_addr_arr_size;
> +
> +if (VIR_ALLOC_N((*(ifaces_ret)[i]).addrs, ip_addr_arr_size) < 0)
> +goto error;
> +
> +for (j = 0; j < ip_addr_arr_size; j++) {
> +virJSONValuePtr ip_addr_obj =
> virJSONValue

[libvirt] [PATCHv3 5/5] domifaddr: Expose python binding

[nehaljwani]

Expose virDomainInterfacesAddresses to python binding

examples/python/Makefile.am:
  * Add new file domipaddrs.py

examples/python/README:
  * Add documentation for the python example

python/libvirt-override-api.xml:
  * Add new symbol for virDomainInterfacesAddresses

python/libvirt-override.c:
  * Hand written python api

---
 examples/python/Makefile.am |   2 +-
 examples/python/README  |   1 +
 examples/python/domipaddrs.py   |  50 ++
 python/libvirt-override-api.xml |   8 ++-
 python/libvirt-override.c   | 114 
 5 files changed, 173 insertions(+), 2 deletions(-)
 create mode 100755 examples/python/domipaddrs.py

diff --git a/examples/python/Makefile.am b/examples/python/Makefile.am
index 2cacfa1..d33ee17 100644
--- a/examples/python/Makefile.am
+++ b/examples/python/Makefile.am
@@ -17,4 +17,4 @@
 EXTRA_DIST=\
README  \
consolecallback.py  \
-   dominfo.py domrestore.py domsave.py domstart.py esxlist.py
+   dominfo.py domrestore.py domsave.py domstart.py esxlist.py domipaddrs.py
diff --git a/examples/python/README b/examples/python/README
index f4db76c..1285d52 100644
--- a/examples/python/README
+++ b/examples/python/README
@@ -10,6 +10,7 @@ domsave.py  - save all running domU's into a directory
 domrestore.py - restore domU's from their saved files in a directory
 esxlist.py  - list active domains of an VMware ESX host and print some info.
   also demonstrates how to use the libvirt.openAuth() method
+domipaddrs.py - print domain interfaces along with their MAC and IP addresses
 
 The XML files in this directory are examples of the XML format that libvirt
 expects, and will have to be adapted for your setup. They are only needed
diff --git a/examples/python/domipaddrs.py b/examples/python/domipaddrs.py
new file mode 100755
index 000..cc16c67
--- /dev/null
+++ b/examples/python/domipaddrs.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# domipaddrds - print domain interfaces along with their MAC and IP addresses
+
+import libvirt
+import sys
+
+def usage():
+print "Usage: %s [URI] DOMAIN" % sys.argv[0]
+print "Print domain interfaces along with their MAC and IP 
addresses"
+
+uri = None
+name = None
+args = len(sys.argv)
+
+if args == 2:
+name = sys.argv[1]
+elif args == 3:
+uri = sys.argv[1]
+name = sys.argv[2]
+else:
+usage()
+sys.exit(2)
+
+conn = libvirt.openReadOnly(uri)
+if conn == None:
+print "Unable to open connection to libvirt"
+sys.exit(1)
+
+try:
+dom = conn.lookupByName(name)
+except libvirt.libvirtError:
+print "Domain %s not found" % name
+sys.exit(0)
+
+ifaces = dom.interfacesAddresses(0)
+if (ifaces == None):
+print "Failed to get domain interfaces"
+sys.exit(0)
+
+print " {0:10} {1:17}{2}".format("Interface", "MAC address", "IP Address")
+
+for (name, val) in ifaces.iteritems():
+print " {0:10} {1:17}".format(name, val['hwaddr']),
+
+if (val['ip_addrs'] <> None):
+print "  ",
+for addr in val['ip_addrs']:
+print "{0}/{1} ".format(addr['addr'], addr['prefix']),
+
+print
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 9a88215..f5c6e83 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -602,5 +602,11 @@
   
   
 
-  
+
+  returns a dictionary of domain interfaces along with their MAC and 
IP addresses
+  
+  
+  
+
+
 
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d16b9a2..50bb7d1 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -4761,6 +4761,119 @@ cleanup:
 return py_retval;
 }
 
+
+static PyObject *
+libvirt_virDomainInterfacesAddresses(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+PyObject *py_retval = VIR_PY_NONE;
+virDomainPtr domain;
+PyObject *pyobj_domain;
+unsigned int flags;
+virDomainInterfacePtr *ifaces = NULL;
+int ifaces_count = 0;
+size_t i, j;
+int ret;
+bool full_free = true;
+
+if (!PyArg_ParseTuple(args, (char *) "Oi:virDomainInterfacePtr",
+  &pyobj_domain, &flags))
+return NULL;
+
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ifaces_count = virDomainInterfacesAddresses(domain, &ifaces, flags);
+ret = ifaces_count;
+LIBVIRT_END_ALLOW_THREADS;
+if (ret < 0)
+goto cleanup;
+
+if (!(py_retval = PyDict_New()))
+goto no_memory;
+
+for (i = 0; i < ifaces_count; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+PyObject *py_ip_addrs = NULL;
+PyObject *py_iface = NULL;
+
+if (!(py_iface = PyDict_New()))
+goto no_memory;
+
+if (iface-

[libvirt] [PATCHv3 3/5] domifaddr: Implement the API for qemu

[nehaljwani]

By querying the qemu guest agent with the QMP command
"guest-network-get-interfaces" and converting the received
JSON output to structured objects.

src/qemu/qemu_agent.h:
  * Define qemuAgentGetInterfaces

src/qemu/qemu_agent.c:
  * Implement qemuAgentGetInterface

src/qemu/qemu_driver.c:
  * New function qemuDomainInterfacesAddresses

src/remote_protocol-sructs:
  * Define new structs

---
 src/qemu/qemu_agent.c  | 157 +
 src/qemu/qemu_agent.h  |   3 +
 src/qemu/qemu_driver.c |  55 +
 tests/qemuagenttest.c  |  90 +++-
 4 files changed, 304 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 2cd0ccc..015e752 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -844,6 +844,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
 VIR_DEBUG("Attempt to send command while error is set %s",
   NULLSTR(mon->lastError.message));
 virSetError(&mon->lastError);
+printf("Upper\n");
 return -1;
 }
 
@@ -862,6 +863,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
 while (!mon->msg->finished) {
 if ((then && virCondWaitUntil(&mon->notify, &mon->parent.lock, then) < 
0) ||
 (!then && virCondWait(&mon->notify, &mon->parent.lock) < 0)) {
+
 if (errno == ETIMEDOUT) {
 virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("Guest agent not available for now"));
@@ -1319,6 +1321,161 @@ cleanup:
 return ret;
 }
 
+
+int
+qemuAgentGetInterfaces(qemuAgentPtr mon,
+   virDomainInterfacePtr **ifaces)
+{
+int ret = -1;
+size_t i, j;
+int size = -1;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr ret_array = NULL;
+int ifaces_count = 0;
+virDomainInterfacePtr *ifaces_ret = NULL;
+
+if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL)))
+   return -1;
+
+if (qemuAgentCommand(mon, cmd, &reply, 
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
+qemuAgentCheckError(cmd, reply) < 0) {
+goto cleanup;
+}
+
+if (!(ret_array = virJSONValueObjectGet(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("qemu agent didn't provide 'return' field"));
+goto cleanup;
+}
+
+if ((size = virJSONValueArraySize(ret_array)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("qemu agent didn't return an array of interfaces"));
+goto cleanup;
+}
+
+ifaces_count = (unsigned int) size;
+
+if (VIR_ALLOC_N(ifaces_ret, size) < 0)
+goto cleanup;
+
+for (i = 0; i < size; i++) {
+virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i);
+virJSONValuePtr ip_addr_arr = NULL;
+const char *name, *hwaddr;
+int ip_addr_arr_size;
+
+if (VIR_ALLOC(ifaces_ret[i]) < 0)
+goto cleanup;
+
+/* Shouldn't happen but doesn't hurt to check neither */
+if (!tmp_iface) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("something has went really wrong"));
+goto error;
+}
+
+/* interface name is required to be presented */
+name = virJSONValueObjectGetString(tmp_iface, "name");
+if (!name) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("qemu agent didn't provide 'name' field"));
+goto error;
+}
+
+if (VIR_STRDUP(ifaces_ret[i]->name, name) < 0)
+goto error;
+
+/* hwaddr might be omitted */
+hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address");
+if (hwaddr && VIR_STRDUP(ifaces_ret[i]->hwaddr, hwaddr) < 0)
+goto error;
+
+/* as well as IP address which - moreover -
+ * can be presented multiple times */
+ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses");
+if (!ip_addr_arr)
+continue;
+
+if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0)
+/* Mmm, empty 'ip-address'? */
+continue;
+
+(*(ifaces_ret)[i]).naddrs = (unsigned int) ip_addr_arr_size;
+
+if (VIR_ALLOC_N((*(ifaces_ret)[i]).addrs, ip_addr_arr_size) < 0)
+goto error;
+
+for (j = 0; j < ip_addr_arr_size; j++) {
+virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j);
+virDomainIPAddressPtr ip_addr = &ifaces_ret[i]->addrs[j];
+const char *type, *addr;
+
+/* Shouldn't happen but doesn't hurt to check neither */
+if (!ip_addr_obj) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("something has went really wrong"));
+goto error;
+}
+
+  

[libvirt] [PATCHv3 1/5] domifaddr: Implement the public API

[nehaljwani]

Define a new API virDomainInterfacesAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.

The API is going to provide multiple methods by flags, e.g.

  * Query guest agent
  * Parse lease file of dnsmasq
  * DHCP snooping

But at this stage, it will only work with guest agent, and flags
won't be supported.

include/libvirt/libvirt.h.in:
  * Define virDomainInterfacesAddresses
  * Define structs virDomainInterface, virDomainIPAddress

python/generator.py:
  * Skip the auto-generation for virDomainInterfacesAddresses

src/driver.h:
  * Define domainInterfacesAddresses

src/libvirt.c:
  * Implement virDomainInterfacesAddresses

src/libvirt_public.syms:
  * Export the new symbol

---
 include/libvirt/libvirt.h.in |  33 +
 python/generator.py  |   3 ++
 src/driver.h |   6 +++
 src/libvirt.c| 110 +++
 src/libvirt_public.syms  |   6 +++
 5 files changed, 158 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a47e33c..deb1e1f 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2044,6 +2044,39 @@ int virDomainGetInterfaceParameters 
(virDomainPtr dom,
  virTypedParameterPtr 
params,
  int *nparams, 
unsigned int flags);
 
+typedef enum {
+VIR_IP_ADDR_TYPE_IPV4,
+VIR_IP_ADDR_TYPE_IPV6,
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_IP_ADDR_TYPE_LAST
+#endif
+} virIPAddrType;
+
+
+typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
+typedef virDomainIPAddress *virDomainIPAddressPtr;
+struct _virDomainInterfaceIPAddress {
+int type;   /* virIPAddrType */
+char *addr; /* IP address */
+int prefix; /* IP address prefix */
+};
+
+typedef struct _virDomainInterface virDomainInterface;
+typedef virDomainInterface *virDomainInterfacePtr;
+struct _virDomainInterface {
+char *name; /* interface name */
+char *hwaddr;   /* hardware address */
+unsigned int naddrs;/* number of items in @addrs */
+virDomainIPAddressPtr addrs;/* array of IP addresses */
+};
+
+int virDomainInterfacesAddresses (virDomainPtr dom,
+  virDomainInterfacePtr **ifaces,
+  unsigned int flags);
+
+void virDomainInterfaceFree (virDomainInterfacePtr iface);
+
 /* Management of domain block devices */
 
 int virDomainBlockPeek (virDomainPtr dom,
diff --git a/python/generator.py b/python/generator.py
index 427cebc..4f3c8ee 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -458,6 +458,7 @@ skip_impl = (
 'virNodeGetMemoryParameters',
 'virNodeSetMemoryParameters',
 'virNodeGetCPUMap',
+'virDomainInterfacesAddresses',
 'virDomainMigrate3',
 'virDomainMigrateToURI3',
 )
@@ -560,6 +561,8 @@ skip_function = (
 "virTypedParamsGetString",
 "virTypedParamsGetUInt",
 "virTypedParamsGetULLong",
+
+"virDomainInterfaceFree", #Only for convenience in C
 )
 
 lxc_skip_function = (
diff --git a/src/driver.h b/src/driver.h
index be64333..8b6182e 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -518,6 +518,11 @@ typedef int
   unsigned int flags);
 
 typedef int
+(*virDrvDomainInterfacesAddresses)(virDomainPtr dom,
+   virDomainInterfacePtr **ifaces,
+   unsigned int flags);
+
+typedef int
 (*virDrvDomainMemoryStats)(virDomainPtr domain,
struct _virDomainMemoryStat *stats,
unsigned int nr_stats,
@@ -1238,6 +1243,7 @@ struct _virDriver {
 virDrvDomainInterfaceStats domainInterfaceStats;
 virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
 virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
+virDrvDomainInterfacesAddressesdomainInterfacesAddresses;
 virDrvDomainMemoryStats domainMemoryStats;
 virDrvDomainBlockPeek domainBlockPeek;
 virDrvDomainMemoryPeek domainMemoryPeek;
diff --git a/src/libvirt.c b/src/libvirt.c
index 07a3fd5..2d4b0bf 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -8643,6 +8643,97 @@ error:
 return -1;
 }
 
+ /**
+ * virDomainInterfacesAddresses:
+ * @dom: domain object
+ * @ifaces: array of @dom interfaces
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Return an array of interfaces present in given domain along with
+ * their IP and MAC addresses. Note that single interface can have
+ * multiple or even 0 IP address.
+ *
+ * This API dynamic

[libvirt] [PATCHv3 2/5] domifaddr: Implement the remote protocol

[nehaljwani]

Implement RPC calls for virDomainInterfacesAddresses

daemon/remote.c
   * Define remoteSerializeDomainInterfacePtr, 
remoteDispatchDomainInterfacesAddresses

src/remote/remote_driver.c
   * Define remoteDomainInterfacesAddresses

src/remote/remote_protocol.x
   * New RPC procedure: REMOTE_PROC_DOMAIN_INTERFACES_ADDRESSES
   * Define structs remote_domain_ip_addr, remote_domain_interface,
 remote_domain_interfaces_addresses_args, 
remote_domain_interfaces_addresses_ret

src/remote_protocol-structs
   * New structs added

---
 daemon/remote.c  | 123 +++
 src/remote/remote_driver.c   |  87 ++
 src/remote/remote_protocol.x |  41 ++-
 src/remote_protocol-structs  |  24 +
 4 files changed, 274 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 03d5557..31640c6 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -5025,7 +5025,130 @@ cleanup:
 return rv;
 }
 
+static int
+remoteSerializeDomainInterfacePtr(virDomainInterfacePtr *ifaces,
+  unsigned int ifaces_count,
+  remote_domain_interfaces_addresses_ret *ret)
+{
+size_t i, j;
+
+if (ifaces_count > REMOTE_DOMAIN_INTERFACE_MAX)
+return -1;
+
+if (VIR_ALLOC_N(ret->ifaces.ifaces_val, ifaces_count) < 0)
+return -1;
+
+ret->ifaces.ifaces_len = ifaces_count;
+
+for (i = 0; i < ifaces_count; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]);
+
+if ((VIR_STRDUP(iface_ret->name, iface->name)) < 0)
+goto cleanup;
+
+if (iface->hwaddr) {
+char **hwaddr_p = NULL;
+if (VIR_ALLOC(hwaddr_p) < 0)
+goto cleanup;
+if (VIR_STRDUP(*hwaddr_p, iface->hwaddr) < 0) {
+VIR_FREE(hwaddr_p);
+goto cleanup;
+}
+
+iface_ret->hwaddr = hwaddr_p;
+}
+
+if (iface->naddrs > REMOTE_DOMAIN_IP_ADDR_MAX)
+goto cleanup;
+
+if (VIR_ALLOC_N(iface_ret->addrs.addrs_val,
+iface->naddrs) < 0)
+goto cleanup;
+
+iface_ret->addrs.addrs_len = iface->naddrs;
+
+for (j = 0; j < iface->naddrs; j++) {
+virDomainIPAddressPtr ip_addr = &(iface->addrs[j]);
+remote_domain_ip_addr *ip_addr_ret =
+&(iface_ret->addrs.addrs_val[j]);
+
+if (VIR_STRDUP(ip_addr_ret->addr, ip_addr->addr) < 0)
+goto cleanup;
+
+ip_addr_ret->prefix = ip_addr->prefix;
+ip_addr_ret->type = ip_addr->type;
+}
+}
+
+return 0;
+
+cleanup:
+if (ret->ifaces.ifaces_val) {
+for (i = 0; i < ifaces_count; i++) {
+remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]);
+VIR_FREE(iface_ret->name);
+VIR_FREE(iface_ret->hwaddr);
+for (j = 0; j < iface_ret->addrs.addrs_len; j++) {
+remote_domain_ip_addr *ip_addr =
+&(iface_ret->addrs.addrs_val[j]);
+VIR_FREE(ip_addr->addr);
+}
+VIR_FREE(iface_ret);
+}
+VIR_FREE(ret->ifaces.ifaces_val);
+}
+
+return -1;
+}
 
+static int
+remoteDispatchDomainInterfacesAddresses(
+virNetServerPtr server ATTRIBUTE_UNUSED,
+virNetServerClientPtr client,
+virNetMessagePtr msg ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_domain_interfaces_addresses_args *args,
+remote_domain_interfaces_addresses_ret *ret)
+{
+int rv = -1;
+virDomainPtr dom = NULL;
+virDomainInterfacePtr *ifaces = NULL;
+int ifaces_count = 0;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv->conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+goto cleanup;
+
+if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, 
args->flags)) < 0)
+goto cleanup;
+
+if (remoteSerializeDomainInterfacePtr(ifaces, ifaces_count, ret) < 0)
+goto cleanup;
+
+rv = 0;
+
+cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+if (ifaces) {
+size_t i;
+for (i = 0; i < ifaces_count; i++) {
+if (ifaces[i])
+virDomainInterfaceFree(ifaces[i]);
+}
+VIR_FREE(ifaces);
+}
+return rv;
+}
 
 /*- Helpers. -*/
 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 71d0034..919adb4 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6477,6 +6477,92 @@ done:
 return rv;
 }
 
+static int
+remoteDomainInterfacesAddresses(virDomainPtr dom,

[libvirt] [PATCHv3 4/5] domifaddr: Add virsh support

[nehaljwani]

Use virDomainInterfacesAddresses in virsh

tools/virsh-domain-monitor.c
   * Introduce new command : domifaddr

   virsh # domifaddr f18
   Name   MAC address  IP address
   ---
   lo 00:00:00:00:00:00127.0.0.1/8 ::1/128
   eth0   52:54:00:89:ad:35192.168.102.142/24 fe80::5054:ff:fe89:ad35/64
   eth1   52:54:00:d3:39:ee192.168.103.183/24 fe80::5054:ff:fed3:39ee/64
   eth2   52:54:00:fe:4c:4f192.168.101.197/24 fe80::5054:ff:fefe:4c4f/64
   eth3   52:54:00:89:4e:97192.168.101.130/24 fe80::5054:ff:fe89:4e97/64

tools/virsh.pod
   * Document new command

---
 tools/virsh-domain-monitor.c | 99 
 tools/virsh.pod  | 10 +
 2 files changed, 109 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index b29b82a..91efa71 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1871,6 +1871,99 @@ cleanup:
 }
 #undef FILTER
 
+/* "domifaddr" command
+ */
+static const vshCmdInfo info_domifaddr[] = {
+{"help", N_("Get network interfaces' addresses for a running domain")},
+{"desc", N_("Get network interfaces' addresses for a running domain")},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_domifaddr[] = {
+{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+{"interface", VSH_OT_DATA, VSH_OFLAG_NONE, N_("network interface name")},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+const char *interface = NULL;
+virDomainInterfacePtr *ifaces = NULL;
+size_t i, j;
+int ifaces_count = 0;
+unsigned int flags = 0;
+bool ret = false;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptString(cmd, "interface", &interface) < 0) {
+goto cleanup;
+}
+
+if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, flags)) < 
0) {
+vshError(ctl, _("Failed to query for interfaces addresses"));
+goto cleanup;
+}
+
+vshPrintExtra(ctl, " %-10s %-17s%s\n%s\n",
+  _("Name"), _("MAC address"), _("IP address"),
+  "---");
+
+for (i = 0; i < ifaces_count; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+const char *hwaddr = "";
+const char *ip_addr_str = NULL;
+
+if (interface && STRNEQ(interface, iface->name)) {
+virBufferFreeAndReset(&buf);
+continue;
+}
+
+if (iface->hwaddr)
+hwaddr = iface->hwaddr;
+
+for (j = 0; j < iface->naddrs; j++) {
+if (j)
+virBufferAddChar(&buf, ' ');
+virBufferAsprintf(&buf, "%s/%d",
+  iface->addrs[j].addr,
+  iface->addrs[j].prefix);
+}
+
+if (virBufferError(&buf)) {
+virBufferFreeAndReset(&buf);
+virReportOOMError();
+return ret;
+}
+
+ip_addr_str = virBufferContentAndReset(&buf);
+
+if (!ip_addr_str)
+ip_addr_str = "";
+
+vshPrintExtra(ctl, " %-10s %-17s%s\n",
+ iface->name, hwaddr, ip_addr_str);
+
+virBufferFreeAndReset(&buf);
+}
+
+ret = true;
+
+cleanup:
+for (i = 0; i < ifaces_count; i++) {
+if (ifaces[i])
+virDomainInterfaceFree(ifaces[i]);
+}
+VIR_FREE(ifaces);
+
+virDomainFree(dom);
+return ret;
+}
+
 const vshCmdDef domMonitoringCmds[] = {
 {.name = "domblkerror",
  .handler = cmdDomBlkError,
@@ -1944,5 +2037,11 @@ const vshCmdDef domMonitoringCmds[] = {
  .info = info_list,
  .flags = 0
 },
+{.name = "domifaddr",
+ .handler = cmdDomIfAddr,
+ .opts = opts_domifaddr,
+ .info = info_domifaddr,
+ .flags = 0
+},
 {.name = NULL}
 };
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0ae5178..008ffea 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -636,6 +636,16 @@ B (fields appear in the following 
order):
   flush_total_times - total time flush operations took (ns)
 <-- other fields provided by hypervisor -->
 
+
+=item B I [I]
+
+Get a list of interfaces of a running domain along with their IP and MAC
+addresses, or limited output just for one interface if I is
+specified. Note, that I  can be driver dependent, it can be
+the name within guest OS or the name you would see in domain XML.
+Moreover, the whole command may require a guest agent to be configured
+for the queried domain under some drivers, notably qemu.
+
 =item B I I
 
 Get network interface stats for a running domain.
-- 
1.7.11.7

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

[libvirt] [PATCHv3 0/5] Introduce API to query IP addresses for given domain

[nehaljwani]

This feature has been requested for a very long time. Since qemu guest
agent gives us reliable results, now the wait is over.

The RFC was first proposed by Michal Privoznik:
 http://www.mail-archive.com/libvir-list@redhat.com/msg51857.html
A patch was submitted, using structs:
 http://www.mail-archive.com/libvir-list@redhat.com/msg57141.html
Another patch was submitted, using XML:
 http://www.mail-archive.com/libvir-list@redhat.com/msg57829.html

Neither of the patches were accepted, probably due to lack of extensibility
and usability. Hence, we thought of using virTypedParameters for reporting
list of interfaces along with their MAC address and IP addresses. The RFC
can be found here:
 http://www.mail-archive.com/libvir-list@redhat.com/msg79793.html

The idea of extensibility was rejected and rendered out of scope of
libvirt. Hence, we were back to structs.

This API is called virDomainInterfacesAddresses which returns a dynamically
allocated array of virDomainInterface struct. The great disadvantage is
once this gets released, it's written in stone and we cannot change
or add an item into it.

The API supports two methods:

* Return information (list of all associated interfaces with MAC address
 and IP addresses) of all of the domain interfaces by default (if
 no interface name is provided)

* Return information for the specified interface (if an interface name
 is provided)

nehaljwani (5):
  domifaddr: Implement the public API
  domifaddr: Implement the remote protocol
  domifaddr: Implement the API for qemu
  domifaddr: Add virsh support
  domifaddr: Expose python binding

 daemon/remote.c | 123 +++
 examples/python/Makefile.am |   2 +-
 examples/python/README  |   1 +
 examples/python/domipaddrs.py   |  50 +
 include/libvirt/libvirt.h.in|  33 +
 python/generator.py |   3 +
 python/libvirt-override-api.xml |   8 +-
 python/libvirt-override.c   | 114 +
 src/driver.h|   6 ++
 src/libvirt.c   | 110 
 src/libvirt_public.syms |   6 ++
 src/qemu/qemu_agent.c   | 157 
 src/qemu/qemu_agent.h   |   3 +
 src/qemu/qemu_driver.c  |  55 ++
 src/remote/remote_driver.c  |  87 ++
 src/remote/remote_protocol.x|  41 ++-
 src/remote_protocol-structs |  24 ++
 tests/qemuagenttest.c   |  90 ++-
 tools/virsh-domain-monitor.c|  99 +
 tools/virsh.pod |  10 +++
 20 files changed, 1018 insertions(+), 4 deletions(-)
 create mode 100755 examples/python/domipaddrs.py

-- 
1.7.11.7

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

Re: [libvirt] [Qemu-devel] pvpanic plans?

[Peter Maydell]

On 22 August 2013 21:09, Anthony Liguori  wrote:
> Paolo Bonzini  writes:
>> Not just that.  Panic notifiers are called in a substantially unknown
>> environment, with locks taken or interrupts already set up.
>
> If you make the panic notify a config space write, then on virtio-pci,
> it's an outb to a fixed offset within a io address that after boot is
> static.
>
> So the address could be stored in a global and accessed without a lock.

Fine for virtio-mmio too, obviously. I have a vague recollection that
config space writes on virtio-s390 are weird though. (would also
be an issue if we wanted to implement the virtio-console "emergency
write" functionality.)

-- PMM

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

Re: [libvirt] pvpanic plans?

[Anthony Liguori]

On Thu, Aug 22, 2013 at 3:36 PM, Laszlo Ersek  wrote:
> On 08/22/13 22:09, Anthony Liguori wrote:
>
>> The difference is that ACPI or platform devices in general are
>> unexpected to be added.  By definition it means that the motherboard has
>> most likely been changed.
>
> You could encounter a new ACPI artifact after simply re-flashing your MB
> with an updated BIOS, without opening the chassis. "If windows can't
> deal with that, their loss!" :)

I'm pretty sure "does Windows boot up okay" is on every major vendor's
firmware test plan for shipping new updates...

Regards,

Anthony Liguori

> Laszlo
> /hides

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

Re: [libvirt] pvpanic plans?

[Laszlo Ersek]

On 08/22/13 22:09, Anthony Liguori wrote:

> The difference is that ACPI or platform devices in general are
> unexpected to be added.  By definition it means that the motherboard has
> most likely been changed.

You could encounter a new ACPI artifact after simply re-flashing your MB
with an updated BIOS, without opening the chassis. "If windows can't
deal with that, their loss!" :)

Laszlo
/hides

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

Re: [libvirt] pvpanic plans?

[Anthony Liguori]

Paolo Bonzini  writes:

> Il 22/08/2013 19:53, Laszlo Ersek ha scritto:
>>> > We should just introduce a simple watchdog device based on virtio and
>>> > call it a day.  Then it's cross platform, solves the guest enumeration
>>> > problem, and libvirt can detect the presence of the new device.
>> If the guest doesn't initialize the proposed virtio-panic device, then
>> it will lie dormant too, just like the current pvpanic device. That's good.
>> 
>> However a new (standalone) virtio device will take up yet another PCI
>> function (a full device if you want it to be hotpluggable). PCI
>> functions are scarcer than ioports.
>
> Not just that.  Panic notifiers are called in a substantially unknown
> environment, with locks taken or interrupts already set up.

If you make the panic notify a config space write, then on virtio-pci,
it's an outb to a fixed offset within a io address that after boot is
static.

So the address could be stored in a global and accessed without a lock.

> This is why we went for a simple ISA device.

"Simple ISA device" doesn't exist outside of x86 and as we are learning,
it's not all that simple.

> Configuration via ACPI
> follows naturally from there, and anyway any other standard of the day
> would have had the same problem with Windows.  At some point we had ACPI
> methods instead of a simple ioport write, but we had to remove that
> because the ACPI subsystem might have had its lock taken.

The difference is that ACPI or platform devices in general are
unexpected to be added.  By definition it means that the motherboard has
most likely been changed.

OTOH, a new PCI device is expected and most OSes will deal gracefully
with it.

>
> Also, a virtio watchdog device makes little sense, IMHO.  PV makes sense
> if emulation has insufficient performance, excessive CPU usage, or
> excessive complexity.  We already have both an ISA and a PCI watchdog,
> and they serve their purpose wonderfully.

Neither of which actually work with modern versions of Windows FWIW.

Plus emulated watchdogs do not take into account steal time or
overcommit in general.  I've seen multiple cases where a naive watchdog
has a problem in the field when the system is under heavy load.

A PV watchdog actually makes sense because it can be defined based on
guest run time instead of wall clock time.

Regards,

Anthony Liguori

>
> Paolo

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

[libvirt] [PATCH] tests: skip schema validation tests if xmllint is missing

[Eric Blake]

On IRC, someone complained that a system without xmllint installed
failed a number of tests.

* tests/schematestutils.sh: Probe for xmllint.

Signed-off-by: Eric Blake 
---
 tests/schematestutils.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh
index e594f04..a0ff77e 100644
--- a/tests/schematestutils.sh
+++ b/tests/schematestutils.sh
@@ -1,5 +1,7 @@
 #!/bin/sh

+(xmllint --version) >/dev/null 2>&1 || skip_test_ 'Missing xmllint'
+
 check_schema () {

 DIRS=$1
-- 
1.8.3.1

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

Re: [libvirt] pvpanic plans?

[Eric Blake]

On 08/22/2013 01:41 PM, Laszlo Ersek wrote:
> On 08/22/13 21:19, Paolo Bonzini wrote:
>> Il 22/08/2013 19:15, Laszlo Ersek ha scritto:
 2) On all versions,  will only work if the element is there.
>>>
>>> I like this, because, if on_crash doesn't work without panic_notifier
>>> *at all*, then we can just drop panic_notifier, and make on_crash mean
>>> (on_crash && panic_notifier) in the original sense.
>>>
>>> IOW, drop "panic_notifier", and make "on_crash" work *always*.
>>
>> No, we cannot because of backwards compatibility.  VMs could have no
>> on_crash element (which means destroy) and yet the
>> guest admin could expect them to reboot on panic.
> 
> Ah. I thought "no on_crash" meant ignore, or
> something like that -- if on_crash was absent, the guest wouldn't see a
> working pvpanic device in ACPI, and wouldn't trigger the event in qemu.

Unfortunately, ignore does not exist in current
libvirt codebase, and  is always present on output (if omitted
on input, it is present as destroy on output; but
MOST vms have it as restart thanks to
virt-install's defaults).

In short, libvirt's problem is that older libvirt basically ignored the
setting (whether default of destroy or set by virt-manager to restart),
BOTH of those common options are most sensibly implemented by having a
panic device, but adding a panic device is guest visible, and therefore
must be controlled by some NEW piece of XML.  If we add
ignorehttp://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemuagenttest.c: Missing documentation (Timeout)

[Eric Blake]

On 08/22/2013 01:43 PM, nehaljwani wrote:
> From: Nehal J Wani 
> 
> In tests/qemuagenttest.c, the Timeout test should always be
> called last. Any additional tests should come before this.

Especially after IRC conversations today, where trying to add a test
after this point led to some head-scratching while debugging.

> 
> ---
>  tests/qemuagenttest.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

ACK and pushed.  Congrats on your first upstream libvirt patch.

> 
> diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
> index e338c98..cf32fc4 100644
> --- a/tests/qemuagenttest.c
> +++ b/tests/qemuagenttest.c
> @@ -605,7 +605,8 @@ mymain(void)
>  DO_TEST(Shutdown);
>  DO_TEST(CPU);
>  DO_TEST(ArbitraryCommand);
> -DO_TEST(Timeout);
> +
> +DO_TEST(Timeout); /*Timeout should always be called last*/

Style nit - we prefer spaces between /* content */

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] pvpanic plans?

[Laszlo Ersek]

On 08/22/13 21:19, Paolo Bonzini wrote:
> Il 22/08/2013 19:15, Laszlo Ersek ha scritto:
>>> 2) On all versions,  will only work if the element is there.
>>
>> I like this, because, if on_crash doesn't work without panic_notifier
>> *at all*, then we can just drop panic_notifier, and make on_crash mean
>> (on_crash && panic_notifier) in the original sense.
>>
>> IOW, drop "panic_notifier", and make "on_crash" work *always*.
> 
> No, we cannot because of backwards compatibility.  VMs could have no
> on_crash element (which means destroy) and yet the
> guest admin could expect them to reboot on panic.

Ah. I thought "no on_crash" meant ignore, or
something like that -- if on_crash was absent, the guest wouldn't see a
working pvpanic device in ACPI, and wouldn't trigger the event in qemu.

>>> 2b) QEMU will provide a way for libvirt to detect that no machine type
>>> has the builtin pvpanic.  If some machine type may have the builtin
>>> pvpanic, and  is absent, libvirt will add
>>> "-global pvpanic.iobase=0" to neutralize it.  Otherwise, libvirt
>>> will create the device normally.
>>>
>>> A possible way for libvirt to detect "good" machine types is a
>>> dummy property.  This is a bit ugly in that the property would not
>>> affect the behavior of the device.  The property would remain in
>>> the long term.
>>>
>>> Another possibility is for QEMU to rename the device, e.g. to
>>> isa-pvpanic.  This is also somewhat gross, but not visible in the
>>> long term when the "pvpanic" name will be lost in history.
>>>
>>> Advantage 1: libvirt has no knowledge of the pvpanic port number
>>>
>>> Disadvantage 1: same as above
>>>
>>> Disadvantage 2: need a somewhat gross change in QEMU
>>>
>>>
>>> This method also provides an (also somewhat gross on the QEMU side)
>>> way to detect other changes in the pvpanic semantics.  One example
>>> mentioned below, is making the panicked state temporary.
>>
>> Too much work in qemu, in order to introduce ugliness, to hide older
>> ugliness.
> 
> Is it too much work?  s/"pvpanic"/"isa-pvpanic"?

... I probably skipped the rename option because you called it gross
(and maybe because I (erroneously?) recall Michael's opposition). I
think I meant the dummy property under "too much work" (it may not be,
in retrospect, but properties always imply compat stuff for me, and
*that* is scary).

Laszlo

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

Re: [libvirt] [Qemu-devel] pvpanic plans?

[Christian Borntraeger]

On 22/08/13 20:33, Anthony Liguori wrote:
> Laszlo Ersek  writes:
> 
>> On 08/22/13 18:10, Paolo Bonzini wrote:
>>> The thread from yesterday has died off (perhaps also because of
>>> my inappropriate answer to Michael, for which I apologize to him
>>> and everyone).  I took some time to discuss the libvirt requirements
>>> further with Daniel Berrange and Eric Blake on IRC.  If anyone is
>>> interested, I can give logs.  This is a suggestion for how to
>>> proceed in both QEMU and libvirt.
>>
>> The analysis is pretty overwhelming :)
>>
>> I have read Anthony's response and I'm not trying to argue -- I'm just
>> spending a few thoughts on this and I'm willing to let them go to waste.
>>
>> In general I think we should minimize the quirks the user (who edits the
>> libvirt XML) has to know about. Interactions between some XML elements,
>> without explicit inter-references (formulated as attributes, like
>> controller/index) are Bad (TM). So,
>>
>>> == Builtin pvpanic ==
>>>
>>> QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
>>> break migration.
>>>
>>>
>>> == Support in libvirt for current functionality ==
>>>
>>> libvirt will add a  element, and possibly a capability
>>> for it accessible via "virsh capabilities".  There are two possibilities:
>>>
>>> 1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
>>>other than pc-1.5),  will only work if the element is there.
>>>On QEMU 1.5.0->1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
>>> will be obeyed always, and may override e.g. reboot-on-panic
>>>if a guest driver exist.
>>
>> I don't like this because there's some interplay between on_crash and
>> panic_notifier, which even depends on the qemu version.
>>
>>
>>>
>>> 2) On all versions,  will only work if the element is there.
>>
>> I like this, because, if on_crash doesn't work without panic_notifier
>> *at all*, then we can just drop panic_notifier, and make on_crash mean
>> (on_crash && panic_notifier) in the original sense.
>>
>> IOW, drop "panic_notifier", and make "on_crash" work *always*.
>>
>>>
>>>
>>> In turn, there are two ways to implement (2):
>>>
>>> 2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
>>> the builtin pvpanic device if present.  
>>> will create the device with -device pvpanic,iobase=0x505
>>>
>>> Advantage: no changes to QEMU
>>>
>>> Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
>>>   and pc-1.5 machine type will write to a pvpanic device instead of
>>>   the DMA controller.  Probably harmless, and limited to some QEMU
>>>   versions.
>>>
>>> Disadvantage 2: libvirt has knowledge of the pvpanic port number
>>
>> Updating this paragraph with my above suggestion:
>>
>> - (s/pvpanic.iobase/pvpanic.ioport/g)
>>
>> - if "on_crash" is absent:
>>   - for 1.{5.0,5.1,5.2,5.3,6.0}, add -global pvpanic.ioport=0
>>   - for other versions, do nothing
>>
>> - if "on_crash" is present:
>>   - for 1.{5.0,5.1,5.2,5.3,6.0}, do nothing,
>>   - for other versions, pass -device pvpanic
>> (knowledge of 0x505 is unneeded)
> 
> Just to further complicate things...
> 
> pvpanic is not going to be present on S390, PPC, ARM, or MIPS because of
> the fact that it's x86 specific.

What about crashed state? I have implemented this state after the guest entered
disabled wait (by convention used by all s390 OSes for crashes)

See commit 08eb8c85e3967b97865d46acadf26dc908fbb094
Author: Christian Borntraeger 
Date:   Fri Apr 26 11:24:47 2013 +0800

Wire up disabled wait a panicked event on s390



Should we remove that as well? From s390 point of view, after allowing
the crashed->running transition the feature would probably work as expected.


Christian

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

[libvirt] [PATCH] qemuagenttest.c: Missing documentation (Timeout)

[nehaljwani]

From: Nehal J Wani 

In tests/qemuagenttest.c, the Timeout test should always be
called last. Any additional tests should come before this.

---
 tests/qemuagenttest.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index e338c98..cf32fc4 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -605,7 +605,8 @@ mymain(void)
 DO_TEST(Shutdown);
 DO_TEST(CPU);
 DO_TEST(ArbitraryCommand);
-DO_TEST(Timeout);
+
+DO_TEST(Timeout); /*Timeout should always be called last*/
 
 virObjectUnref(xmlopt);
 
-- 
1.7.11.7

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

Re: [libvirt] pvpanic plans?

[Paolo Bonzini]

Il 22/08/2013 19:15, Laszlo Ersek ha scritto:
>> 2) On all versions,  will only work if the element is there.
> 
> I like this, because, if on_crash doesn't work without panic_notifier
> *at all*, then we can just drop panic_notifier, and make on_crash mean
> (on_crash && panic_notifier) in the original sense.
> 
> IOW, drop "panic_notifier", and make "on_crash" work *always*.

No, we cannot because of backwards compatibility.  VMs could have no
on_crash element (which means destroy) and yet the
guest admin could expect them to reboot on panic.


>> 2b) QEMU will provide a way for libvirt to detect that no machine type
>> has the builtin pvpanic.  If some machine type may have the builtin
>> pvpanic, and  is absent, libvirt will add
>> "-global pvpanic.iobase=0" to neutralize it.  Otherwise, libvirt
>> will create the device normally.
>>
>> A possible way for libvirt to detect "good" machine types is a
>> dummy property.  This is a bit ugly in that the property would not
>> affect the behavior of the device.  The property would remain in
>> the long term.
>>
>> Another possibility is for QEMU to rename the device, e.g. to
>> isa-pvpanic.  This is also somewhat gross, but not visible in the
>> long term when the "pvpanic" name will be lost in history.
>>
>> Advantage 1: libvirt has no knowledge of the pvpanic port number
>>
>> Disadvantage 1: same as above
>>
>> Disadvantage 2: need a somewhat gross change in QEMU
>>
>>
>> This method also provides an (also somewhat gross on the QEMU side)
>> way to detect other changes in the pvpanic semantics.  One example
>> mentioned below, is making the panicked state temporary.
> 
> Too much work in qemu, in order to introduce ugliness, to hide older
> ugliness.

Is it too much work?  s/"pvpanic"/"isa-pvpanic"?

Paolo

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

[libvirt] [PATCH 5/5] cpu_models: add Python bindings

[Giuseppe Scrivano]

Signed-off-by: Giuseppe Scrivano 
---
 python/generator.py |  2 +-
 python/libvirt-override-api.xml |  7 ++
 python/libvirt-override.c   | 56 +
 python/libvirt-override.py  | 11 
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/python/generator.py b/python/generator.py
index f8ac100..10d4a49 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -250,6 +250,7 @@ lxc_functions_failed = []
 qemu_functions_failed = []
 functions_skipped = [
 "virConnectListDomains",
+"virConnectGetCPUModelNames",
 ]
 lxc_functions_skipped = []
 qemu_functions_skipped = []
@@ -366,7 +367,6 @@ foreign_encoding_args = (
 # Class methods which are written by hand in libvirt.c but the Python-level
 # code is still automatically generated (so they are not in skip_function()).
 skip_impl = (
-"virConnectGetCPUModelNames",
 'virConnectGetVersion',
 'virConnectGetLibVersion',
 'virConnectListDomainsID',
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 9a88215..1bceb05 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -476,6 +476,13 @@
   
   
 
+
+  Get the list of supported CPU models.
+  
+  
+  
+  
+
 
   collect the list of snapshot names for the given domain
   
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d16b9a2..8536561 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -2276,6 +2276,62 @@ libvirt_virConnectGetVersion(PyObject *self 
ATTRIBUTE_UNUSED,
 return PyInt_FromLong(hvVersion);
 }
 
+PyObject *
+libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED,
+   PyObject *args)
+{
+int c_retval;
+virConnectPtr conn;
+PyObject *rv, *pyobj_conn;
+char **it, **models = NULL;
+int flags = 0;
+const char *arch = NULL;
+unsigned int len;
+
+if (!PyArg_ParseTuple(args, (char *)"Osi:virConnectGetCPUModelNames",
+  &pyobj_conn, &arch, &flags))
+return NULL;
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+
+c_retval = virConnectGetCPUModelNames(conn, arch, &models, flags);
+
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_retval == -1)
+return VIR_PY_INT_FAIL;
+
+len = 0;
+for (it = models; *it; it++)
+len++;
+
+if ((rv = PyList_New(len)) == NULL)
+goto error;
+
+len = 0;
+for (it = models; *it; it++){
+PyObject *str;
+if ((str = PyString_FromString(*it)) == NULL)
+goto error;
+
+PyList_SET_ITEM(rv, len++, str);
+}
+
+done:
+if (models) {
+for (it = models; *it; it++)
+VIR_FREE(*it);
+VIR_FREE(models);
+}
+
+return rv;
+
+error:
+rv = VIR_PY_INT_FAIL;
+goto done;
+}
+
 static PyObject *
 libvirt_virConnectGetLibVersion(PyObject *self ATTRIBUTE_UNUSED,
 PyObject *args)
diff --git a/python/libvirt-override.py b/python/libvirt-override.py
index ccfec48..3471a43 100644
--- a/python/libvirt-override.py
+++ b/python/libvirt-override.py
@@ -207,3 +207,14 @@ def virEventAddTimeout(timeout, cb, opaque):
 ret = libvirtmod.virEventAddTimeout(timeout, cbData)
 if ret == -1: raise libvirtError ('virEventAddTimeout() failed')
 return ret
+
+def getCPUModelNames(conn, arch, flags=0):
+"""
+get the list of supported CPU models.
+@conn: virConnect connection
+@arch: Architecture
+@flags: extra flags; not used yet, so callers should always pass 0.
+"""
+ret = libvirtmod.virConnectGetCPUModelNames(conn._o, arch, flags)
+if ret == None: raise libvirtError ('virConnectGetCPUModelNames() failed', 
conn=self)
+return ret
-- 
1.8.3.1

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

[libvirt] [PATCH 4/5] cpu_models: add the support for the test protocol

[Giuseppe Scrivano]

Signed-off-by: Giuseppe Scrivano 
---
 src/test/test_driver.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d7b2e40..9b73532 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5801,6 +5801,21 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
 return ret;
 }
 
+static int
+testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED,
+const char *arch ATTRIBUTE_UNUSED,
+char ***models,
+unsigned int flags)
+{
+char **null_list;
+
+virCheckFlags(0, -1);
+if (VIR_ALLOC_N(null_list, 1) < 0)
+return -1;
+
+*models = null_list;
+return 0;
+}
 
 static virDriver testDriver = {
 .no = VIR_DRV_TEST,
@@ -5872,6 +5887,7 @@ static virDriver testDriver = {
 .connectIsAlive = testConnectIsAlive, /* 0.9.8 */
 .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */
 .domainScreenshot = testDomainScreenshot, /* 1.0.5 */
+.connectGetCPUModelNames = testConnectGetCPUModelNames, /* 1.1.2 */
 };
 
 static virNetworkDriver testNetworkDriver = {
-- 
1.8.3.1

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

[libvirt] [PATCH 2/5] cpu_models: implement the remote protocol

[Giuseppe Scrivano]

Signed-off-by: Giuseppe Scrivano 
---
 daemon/remote.c  | 37 
 src/remote/remote_driver.c   | 51 
 src/remote/remote_protocol.x | 17 ++-
 src/remote_protocol-structs  | 11 ++
 4 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 03d5557..e33f616 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4923,6 +4923,43 @@ cleanup:
 }
 
 
+static int
+remoteDispatchConnectGetCPUModelNames(
+virNetServerPtr server ATTRIBUTE_UNUSED,
+virNetServerClientPtr client ATTRIBUTE_UNUSED,
+virNetMessagePtr msg ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_connect_get_cpu_model_names_args *args,
+remote_connect_get_cpu_model_names_ret *ret)
+{
+int rv = -1;
+char **models;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv->conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+goto cleanup;
+}
+
+if (virConnectGetCPUModelNames(priv->conn, args->arch, &models,
+   args->flags) < 0)
+goto cleanup;
+
+ret->models.models_val = models;
+ret->models.models_len = 0;
+while (*models++)
+ret->models.models_len++;
+
+rv = 0;
+
+cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+return rv;
+}
+
+
 static int remoteDispatchDomainCreateXMLWithFiles(
 virNetServerPtr server ATTRIBUTE_UNUSED,
 virNetServerClientPtr client,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 71d0034..eb0bca2 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5485,6 +5485,56 @@ done:
 
 
 static int
+remoteConnectGetCPUModelNames(virConnectPtr conn,
+  const char *arch,
+  char ***models,
+  unsigned int flags)
+{
+int rv = -1;
+size_t i;
+char **retmodels;
+remote_connect_get_cpu_model_names_args args;
+remote_connect_get_cpu_model_names_ret ret;
+struct private_data *priv = conn->privateData;
+remoteDriverLock(priv);
+
+memset(&args, 0, sizeof(args));
+memset(&ret, 0, sizeof(ret));
+
+args.arch = (char *) arch;
+args.flags = flags;
+
+if (call(conn, priv, 0, REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES,
+ (xdrproc_t) xdr_remote_connect_get_cpu_model_names_args,
+ (char *) &args,
+ (xdrproc_t) xdr_remote_connect_get_cpu_model_names_ret,
+ (char *) &ret) < 0)
+goto error;
+
+if (VIR_ALLOC_N(retmodels, ret.models.models_len + 1) < 0)
+goto error;
+
+for (i = 0; i < ret.models.models_len; i++)
+retmodels[i] = ret.models.models_val[i];
+
+/* Caller frees MODELS.  */
+*models = retmodels;
+rv = 0;
+
+done:
+remoteDriverUnlock(priv);
+return rv;
+
+error:
+rv = -1;
+for (i = 0; i < ret.models.models_len; i++)
+VIR_FREE(ret.models.models_val[i]);
+VIR_FREE(ret.models.models_val);
+goto done;
+}
+
+
+static int
 remoteDomainOpenGraphics(virDomainPtr dom,
  unsigned int idx,
  int fd,
@@ -6811,6 +6861,7 @@ static virDriver remote_driver = {
 .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 
*/
 .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */
 .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 
*/
+.connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.2 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 7cfebdf..924d629 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2837,6 +2837,15 @@ struct remote_domain_event_device_removed_msg {
 remote_nonnull_string devAlias;
 };
 
+struct remote_connect_get_cpu_model_names_args {
+remote_nonnull_string arch;
+unsigned int flags;
+};
+
+struct remote_connect_get_cpu_model_names_ret {
+remote_nonnull_string models<>;
+};
+
 /*- Protocol. -*/
 
 /* Define the program number, protocol version and procedure numbers here. */
@@ -5000,5 +5009,11 @@ enum remote_procedure {
  * @generate: both
  * @acl: none
  */
-REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311
+REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311,
+
+/**
+ * @generate: none
+ * @acl: connect:read
+ */
+REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES = 312
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 4e27aae..9148202 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2316,6 +2316,16 @@ struct remote_domain_event_device_removed_msg {
 remote_nonnull_domain  dom;
 remo

[libvirt] [PATCH 0/5] add new API virConnectGetCPUModelNames

[Giuseppe Scrivano]

Giuseppe Scrivano (5):
  cpu_models: add new public API
  cpu_models: implement the remote protocol
  cpu_models: add the support for qemu
  cpu_models: add the support for the test protocol
  cpu_models: add Python bindings

 daemon/remote.c | 37 ++
 include/libvirt/libvirt.h.in| 18 +++
 python/generator.py |  1 +
 python/libvirt-override-api.xml |  7 +
 python/libvirt-override.c   | 56 +
 python/libvirt-override.py  | 11 +++
 src/cpu/cpu.c   | 69 +
 src/cpu/cpu.h   |  3 ++
 src/driver.h|  7 +
 src/libvirt.c   | 47 
 src/libvirt_private.syms|  1 +
 src/libvirt_public.syms |  5 +++
 src/qemu/qemu_driver.c  | 14 +
 src/remote/remote_driver.c  | 51 ++
 src/remote/remote_protocol.x| 17 +-
 src/remote_protocol-structs | 11 +++
 src/test/test_driver.c  | 16 ++
 tools/virsh-host.c  | 48 
 tools/virsh.pod |  5 +++
 19 files changed, 423 insertions(+), 1 deletion(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 1/5] cpu_models: add new public API

[Giuseppe Scrivano]

The new function virConnectGetCPUModelNames allows to retrieve the list
of CPU models known by the hypervisor for a specific architecture.

Signed-off-by: Giuseppe Scrivano 
---
It explicitly disables the Python bindings as generator.py doesn't deal
correctly with the new API.  Patch 5 in this series will implement the Python
bindings.

 include/libvirt/libvirt.h.in | 18 
 python/generator.py  |  1 +
 src/cpu/cpu.c| 69 
 src/cpu/cpu.h|  3 ++
 src/driver.h |  7 +
 src/libvirt.c| 47 ++
 src/libvirt_private.syms |  1 +
 src/libvirt_public.syms  |  5 
 tools/virsh-host.c   | 48 ++
 tools/virsh.pod  |  5 
 10 files changed, 204 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a47e33c..43fb738 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4006,6 +4006,24 @@ int virConnectCompareCPU(virConnectPtr conn,
  const char *xmlDesc,
  unsigned int flags);
 
+/**
+ * virConnectGetCPUModelNames:
+ *
+ * @conn: virConnect connection
+ * @arch: Architecture
+ * @models: NULL terminated array of the CPU models supported for the specified
+ * architecture.  Each element and the array itself must be freed by the caller
+ * with free.
+ * @flags: extra flags; not used yet, so callers should always pass 0.
+ *
+ * Get the list of supported CPU models for a specific architecture.
+ *
+ * Returns -1 on error, 0 on success.
+ */
+int virConnectGetCPUModelNames(virConnectPtr conn,
+   const char *arch,
+   char ***models,
+   unsigned int flags);
 
 /**
  * virConnectBaselineCPUFlags
diff --git a/python/generator.py b/python/generator.py
index 427cebc..f8ac100 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -366,6 +366,7 @@ foreign_encoding_args = (
 # Class methods which are written by hand in libvirt.c but the Python-level
 # code is still automatically generated (so they are not in skip_function()).
 skip_impl = (
+"virConnectGetCPUModelNames",
 'virConnectGetVersion',
 'virConnectGetLibVersion',
 'virConnectListDomainsID',
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 023ce26..6abb173 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -27,6 +27,7 @@
 #include "viralloc.h"
 #include "virxml.h"
 #include "cpu.h"
+#include "cpu_map.h"
 #include "cpu_x86.h"
 #include "cpu_powerpc.h"
 #include "cpu_s390.h"
@@ -456,3 +457,71 @@ cpuModelIsAllowed(const char *model,
 }
 return false;
 }
+
+struct cpuGetModelsData
+{
+char **data;
+unsigned int used;
+unsigned int len;  /* Doesn't count the last element DATA (NULL). */
+};
+
+static int
+cpuGetArchModelsCb(enum cpuMapElement element,
+   xmlXPathContextPtr ctxt,
+   void *cbdata)
+{
+char *name;
+struct cpuGetModelsData *data = cbdata;
+if (element != CPU_MAP_ELEMENT_MODEL)
+return 0;
+
+name = virXPathString("string(@name)", ctxt);
+if (name == NULL)
+return -1;
+
+if (data->used == data->len) {
+data->len *= 2;
+if (VIR_REALLOC_N(data->data, data->len + 1) < 0)
+return -1;
+}
+
+data->data[data->used++] = name;
+data->data[data->used] = NULL;
+return 0;
+}
+
+
+static int
+cpuGetArchModels(const char *arch, struct cpuGetModelsData *data)
+{
+return cpuMapLoad(arch, cpuGetArchModelsCb, data);
+}
+
+
+int
+cpuGetModels(const char *arch, char ***models)
+{
+struct cpuGetModelsData data;
+
+*models = data.data = NULL;
+data.used = 0;
+data.len = 8;
+
+if (VIR_ALLOC_N(data.data, data.len + 1) < 0)
+goto error;
+
+if (cpuGetArchModels(arch, &data) < 0)
+goto error;
+
+*models = data.data;
+return 0;
+
+error:
+if (data.data) {
+char **it;
+for (it = data.data; *it; it++)
+VIR_FREE(*it);
+VIR_FREE(data.data);
+}
+return -1;
+}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 7f1d4bd..ba22919 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -175,4 +175,7 @@ cpuModelIsAllowed(const char *model,
   const char **models,
   unsigned int nmodels);
 
+extern int
+cpuGetModels(const char *arch, char ***models);
+
 #endif /* __VIR_CPU_H__ */
diff --git a/src/driver.h b/src/driver.h
index be64333..8cd164a 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -682,6 +682,12 @@ typedef char *
 unsigned int flags);
 
 typedef int
+(*virDrvConnectGetCPUModelNames)(virConnectPtr conn,
+ const char *args,
+ char ***models,
+ unsigned int flags);
+
+typedef int
 (*virDrvD

[libvirt] [PATCH 3/5] cpu_models: add the support for qemu

[Giuseppe Scrivano]

Signed-off-by: Giuseppe Scrivano 
---
 src/qemu/qemu_driver.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0a8e518..cb61243 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16035,6 +16035,19 @@ qemuNodeSuspendForDuration(virConnectPtr conn,
 return nodeSuspendForDuration(target, duration, flags);
 }
 
+static int
+qemuConnectGetCPUModelNames(virConnectPtr conn,
+const char *arch,
+char ***models,
+unsigned int flags)
+{
+virCheckFlags(0, -1);
+if (virConnectGetCPUModelNamesEnsureACL(conn) < 0)
+return -1;
+
+return cpuGetModels(arch, models);
+}
+
 
 static virDriver qemuDriver = {
 .no = VIR_DRV_QEMU,
@@ -16222,6 +16235,7 @@ static virDriver qemuDriver = {
 .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */
 .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */
 .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */
+.connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.2 */
 };
 
 
-- 
1.8.3.1

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

Re: [libvirt] pvpanic plans?

[Paolo Bonzini]

Il 22/08/2013 19:53, Laszlo Ersek ha scritto:
>> > We should just introduce a simple watchdog device based on virtio and
>> > call it a day.  Then it's cross platform, solves the guest enumeration
>> > problem, and libvirt can detect the presence of the new device.
> If the guest doesn't initialize the proposed virtio-panic device, then
> it will lie dormant too, just like the current pvpanic device. That's good.
> 
> However a new (standalone) virtio device will take up yet another PCI
> function (a full device if you want it to be hotpluggable). PCI
> functions are scarcer than ioports.

Not just that.  Panic notifiers are called in a substantially unknown
environment, with locks taken or interrupts already set up.

This is why we went for a simple ISA device.  Configuration via ACPI
follows naturally from there, and anyway any other standard of the day
would have had the same problem with Windows.  At some point we had ACPI
methods instead of a simple ioport write, but we had to remove that
because the ACPI subsystem might have had its lock taken.

Also, a virtio watchdog device makes little sense, IMHO.  PV makes sense
if emulation has insufficient performance, excessive CPU usage, or
excessive complexity.  We already have both an ISA and a PCI watchdog,
and they serve their purpose wonderfully.

Paolo

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

[libvirt] [PATCH 1/2] Add http protocol support for cdrom disk

[Aline Manera]

From: Aline Manera 

QEMU/KVM already allows a HTTP URL for the cdrom ISO image so add this support
to libvirt as well.
The xml should be as following:


  

  


Signed-off-by: Aline Manera 
---
 docs/formatdomain.html.in  |8 +
 docs/schemas/domaincommon.rng  |1 +
 src/conf/domain_conf.c |3 +-
 src/conf/domain_conf.h |1 +
 src/qemu/qemu_command.c|7 
 .../qemuxml2argv-disk-cdrom-network-http.args  |6 
 .../qemuxml2argv-disk-cdrom-network-http.xml   |   37 
 tests/qemuxml2argvtest.c   |2 ++
 8 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9d12293..7df6042 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1501,6 +1501,14 @@
   
   
 
+
+  
+  
+
+  
+  
+  
+
 
   
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index dfcd61c..a69439d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1211,6 +1211,7 @@
 sheepdog
 gluster
 iscsi
+http
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a187a6..8a345e8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -261,7 +261,8 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, 
VIR_DOMAIN_DISK_PROTOCOL_LAST,
   "rbd",
   "sheepdog",
   "gluster",
-  "iscsi")
+  "iscsi",
+  "http")
 
 VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
   "tcp",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 500a5be..77fa00c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -540,6 +540,7 @@ enum virDomainDiskProtocol {
 VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
 VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
 VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
+VIR_DOMAIN_DISK_PROTOCOL_HTTP,
 
 VIR_DOMAIN_DISK_PROTOCOL_LAST
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b5ac15a..f2a9a97 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3826,6 +3826,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 virBufferEscape(&opt, ',', ",", "%s,", disk->src);
 }
 break;
+
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP:
+virBufferAsprintf(&opt, "file=http://%s:%s";,
+  disk->hosts->name,
+  disk->hosts->port ? disk->hosts->port : 
"80");
+virBufferEscape(&opt, ',', ",", "%s,", disk->src);
+break;
 }
 } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
 if (qemuBuildVolumeString(conn, disk, &opt) < 0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
new file mode 100644
index 000..97f5406
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/kvm -S \
+-M pc-1.2 -m 1024 -smp 1 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -boot d -usb \
+-drive 
file=http://host.name:80/url/path/file.iso,if=none,media=cdrom,id=drive-ide0-1-0
 \
+-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml
new file mode 100644
index 000..8c9bc8a
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml
@@ -0,0 +1,37 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  1048576
+  1048576
+  1
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/bin/kvm
+
+  
+  
+
+  
+  
+  
+  
+  
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3a3c304..de93e77 100644
--- a/tests

[libvirt] [PATCH 0/2] Add http and ftp support for cdrom disk

[Aline Manera]

From: Aline Manera 

Aline Manera (2):
  Add http protocol support for cdrom disk
  Add ftp protocol support for cdrom disk

 docs/formatdomain.html.in  |   16 +
 docs/schemas/domaincommon.rng  |2 ++
 src/conf/domain_conf.c |4 ++-
 src/conf/domain_conf.h |2 ++
 src/qemu/qemu_command.c|   13 +++
 .../qemuxml2argv-disk-cdrom-network-ftp.args   |6 
 .../qemuxml2argv-disk-cdrom-network-ftp.xml|   37 
 .../qemuxml2argv-disk-cdrom-network-http.args  |6 
 .../qemuxml2argv-disk-cdrom-network-http.xml   |   37 
 tests/qemuxml2argvtest.c   |4 +++
 10 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml

-- 
1.7.10.4

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

[libvirt] [PATCH 2/2] Add ftp protocol support for cdrom disk

[Aline Manera]

From: Aline Manera 

The ftp protocol is already recognized by qemu/KVM so add this support to
libvirt as well.
The xml should be as following:

 
   
 
   
 

Signed-off-by: Aline Manera 
---
 docs/formatdomain.html.in  |8 +
 docs/schemas/domaincommon.rng  |1 +
 src/conf/domain_conf.c |3 +-
 src/conf/domain_conf.h |1 +
 src/qemu/qemu_command.c|6 
 .../qemuxml2argv-disk-cdrom-network-ftp.args   |6 
 .../qemuxml2argv-disk-cdrom-network-ftp.xml|   37 
 tests/qemuxml2argvtest.c   |2 ++
 8 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7df6042..7f057ec 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1509,6 +1509,14 @@
   
   
 
+
+  
+  
+
+  
+  
+  
+
 
   
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a69439d..0273026 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1212,6 +1212,7 @@
 gluster
 iscsi
 http
+ftp
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a345e8..ffd7ac7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -262,7 +262,8 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, 
VIR_DOMAIN_DISK_PROTOCOL_LAST,
   "sheepdog",
   "gluster",
   "iscsi",
-  "http")
+  "http",
+  "ftp")
 
 VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
   "tcp",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 77fa00c..6cb2d85 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -541,6 +541,7 @@ enum virDomainDiskProtocol {
 VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
 VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
 VIR_DOMAIN_DISK_PROTOCOL_HTTP,
+VIR_DOMAIN_DISK_PROTOCOL_FTP,
 
 VIR_DOMAIN_DISK_PROTOCOL_LAST
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2a9a97..d8e5b47 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3833,6 +3833,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
   disk->hosts->port ? disk->hosts->port : 
"80");
 virBufferEscape(&opt, ',', ",", "%s,", disk->src);
 break;
+case VIR_DOMAIN_DISK_PROTOCOL_FTP:
+virBufferAsprintf(&opt, "file=ftp://%s:%s";,
+  disk->hosts->name,
+  disk->hosts->port ? disk->hosts->port : 
"21");
+virBufferEscape(&opt, ',', ",", "%s,", disk->src);
+break;
 }
 } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
 if (qemuBuildVolumeString(conn, disk, &opt) < 0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.args
new file mode 100644
index 000..4fae2b0
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/kvm -S \
+-M pc-1.2 -m 1024 -smp 1 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -boot d -usb \
+-drive 
file=ftp://host.name:21/url/path/file.iso,if=none,media=cdrom,id=drive-ide0-1-0 
\
+-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
new file mode 100644
index 000..660dedf
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
@@ -0,0 +1,37 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  1048576
+  1048576
+  1
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/bin/kvm
+
+  
+  
+
+  
+  
+  
+  
+  
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index de9

Re: [libvirt] pvpanic plans?

[Anthony Liguori]

Laszlo Ersek  writes:

> On 08/22/13 18:10, Paolo Bonzini wrote:
>> The thread from yesterday has died off (perhaps also because of
>> my inappropriate answer to Michael, for which I apologize to him
>> and everyone).  I took some time to discuss the libvirt requirements
>> further with Daniel Berrange and Eric Blake on IRC.  If anyone is
>> interested, I can give logs.  This is a suggestion for how to
>> proceed in both QEMU and libvirt.
>
> The analysis is pretty overwhelming :)
>
> I have read Anthony's response and I'm not trying to argue -- I'm just
> spending a few thoughts on this and I'm willing to let them go to waste.
>
> In general I think we should minimize the quirks the user (who edits the
> libvirt XML) has to know about. Interactions between some XML elements,
> without explicit inter-references (formulated as attributes, like
> controller/index) are Bad (TM). So,
>
>> == Builtin pvpanic ==
>> 
>> QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
>> break migration.
>> 
>> 
>> == Support in libvirt for current functionality ==
>> 
>> libvirt will add a  element, and possibly a capability
>> for it accessible via "virsh capabilities".  There are two possibilities:
>> 
>> 1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
>>other than pc-1.5),  will only work if the element is there.
>>On QEMU 1.5.0->1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
>> will be obeyed always, and may override e.g. reboot-on-panic
>>if a guest driver exist.
>
> I don't like this because there's some interplay between on_crash and
> panic_notifier, which even depends on the qemu version.
>
>
>> 
>> 2) On all versions,  will only work if the element is there.
>
> I like this, because, if on_crash doesn't work without panic_notifier
> *at all*, then we can just drop panic_notifier, and make on_crash mean
> (on_crash && panic_notifier) in the original sense.
>
> IOW, drop "panic_notifier", and make "on_crash" work *always*.
>
>> 
>> 
>> In turn, there are two ways to implement (2):
>> 
>> 2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
>> the builtin pvpanic device if present.  
>> will create the device with -device pvpanic,iobase=0x505
>> 
>> Advantage: no changes to QEMU
>> 
>> Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
>>   and pc-1.5 machine type will write to a pvpanic device instead of
>>   the DMA controller.  Probably harmless, and limited to some QEMU
>>   versions.
>> 
>> Disadvantage 2: libvirt has knowledge of the pvpanic port number
>
> Updating this paragraph with my above suggestion:
>
> - (s/pvpanic.iobase/pvpanic.ioport/g)
>
> - if "on_crash" is absent:
>   - for 1.{5.0,5.1,5.2,5.3,6.0}, add -global pvpanic.ioport=0
>   - for other versions, do nothing
>
> - if "on_crash" is present:
>   - for 1.{5.0,5.1,5.2,5.3,6.0}, do nothing,
>   - for other versions, pass -device pvpanic
> (knowledge of 0x505 is unneeded)

Just to further complicate things...

pvpanic is not going to be present on S390, PPC, ARM, or MIPS because of
the fact that it's x86 specific.

That means at some point there's going to have to be another device to
support these platforms and libvirt will need to deal with that too.

Regards,

Anthony Liguori

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

Re: [libvirt] pvpanic plans?

[Anthony Liguori]

Laszlo Ersek  writes:

> On 08/22/13 18:44, Anthony Liguori wrote:
>
>> pvpanic has been a failure.  It's a poorly designed device with even
>> worse semantics.
>
> I disagree somewhat.
>
> Requiring a separate ioport is not ideal, I admit. Configuration over
> ACPI is good OTOH (it seems to put standards to good use anyway).
>
> Noone realized pvpanic had poor technical design until the Windows "new
> device" wizard popped up -- is that correct? Most of us are probably not
> habitual Windows users, which is probably why we haven't thought of it
> earlier.

Generating ACPI tables dynamically is painful and worse yet, it's 100%
ACPI specific.  Had we used virtio from the start, we would have had a
cross-architecture mechanism instead of a one-off x86-ism.

Yes, hind sight is 20/20 but that shouldn't stop us from doing things
right when presented the opportunity.

> Maybe we shouldn't promise "there won't be guest-visible changes in ACPI
> contents". If we do promise, maybe we should then make the SeaBIOS
> binary that we're loading dependent on -M too too.
>
> After all, had we managed to completely hide the \_SB.PCI0.ISA.PEVT
> device programmatically, as opposed to only disabling it, we might have
> never realized pvpanic had poor design. Which (almost) means it wouldn't
> have had one.
>
> If we selected a SeaBIOS binary based on -M, then we could hide this
> stuff from Windows.

Yes, at some point I'm sure we'll hit the need for maintaining multiple
copies of SeaBIOS but that's going to make testing all that much
harder.  The longer we can avoid it the better IMHO.

>> I applied it and I'll take the fault for merging it in
>> the first place.
>> 
>> We should simply scrap it and start over.
>
> That will kinda Eff some downstreams in the A...

If it's too late then we're stuck with it, but perhaps some of the
downstreams can skip up about what level of support they need for the
existing device in a bit more detail...

AFAICT, we've got something that's fundamentally broken right now so
downstreams are already in a bind if they're planning on supporting this
device.

>> It has so few users at this
>> point that this is still a realistic option.  Using something based on
>> ISA that requires specific ACPI entries was a mistake.
>> 
>> We should just introduce a simple watchdog device based on virtio and
>> call it a day.  Then it's cross platform, solves the guest enumeration
>> problem, and libvirt can detect the presence of the new device.
>
> If the guest doesn't initialize the proposed virtio-panic device, then
> it will lie dormant too, just like the current pvpanic device. That's
> good.

Ack.

> However a new (standalone) virtio device will take up yet another PCI
> function (a full device if you want it to be hotpluggable). PCI
> functions are scarcer than ioports.

We can always use bridges to expand the number of slots available.  If
we truly run into a situation where slots become too scarce, then we can
look at introducing a PCI-to-N-virtio-devices bridge.

> It will need documentation in the virtio-spec as well.

Ack.

> We'd need an arbitrarily heavily multiplexed paravirt channel between
> guest and qemu. Maybe a dedicated virtio-serial port that's not exposed
> to other host processes; one that qemu would "consume" itself.

I don't think using virtio-serial would be a good approach.

If we make the panic flag a config space variable, it makes it pretty
easy for firmware to use since it's still just an ioport write.

> If you want to be able to panic in boot firmware, writing to an ioport
> is easier than adding a new virtio driver (virtio-serial, or a
> completely new device).

See above.

>> None of the plans outlined below give us a proper solution.  I think
>> removing is our best option at this point.
>
> I'm just trolling ^W playing the devil's advocate here, giving you more
> opportunity to argue your point :)

It's really a tremendously simple virtio device to start with.  No
queues, just a configuration space with a single flag.

Down the road, I can imagine lots of useful extensions like the ability
to send a stack trace to the host as part of the panic.  That would be
mighty handy.  That's easy to add with virtio but pretty much impossible
with the current device.

Plus adding watchdog functionality would be a logical extension too.  I
believe that the watchdogs we emulate today are not supported by a
majority of guests.  A virtio-ras device with Windows and Linux drivers
would give us a universally supported watchdog device.

Regards,

Anthony Liguori

>
> Thanks,
> Laszlo

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

Re: [libvirt] [PATCH] Allow a HTTP URL for cdrom ISO image

[Aline Manera]

On 08/22/2013 10:01 AM, Daniel P. Berrange wrote:

On Wed, Aug 21, 2013 at 04:31:03PM -0300, Aline Manera wrote:

From: Aline Manera 

QEMU/KVM already allows an HTTP URL for the cdrom ISO image so add this support
to libvirt as well.
The xml should be as following:

 
   
 
   
 

Signed-off-by: Aline Manera 
---
  src/conf/domain_conf.c |3 +-
  src/conf/domain_conf.h |1 +
  src/qemu/qemu_command.c|7 
  .../qemuxml2argv-disk-cdrom-network.args   |5 +++
  .../qemuxml2argv-disk-cdrom-network.xml|   37 
  tests/qemuxml2argvtest.c   |2 ++
  6 files changed, 54 insertions(+), 1 deletion(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ea49d2c..461cc95 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -261,7 +261,8 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, 
VIR_DOMAIN_DISK_PROTOCOL_LAST,
"rbd",
"sheepdog",
"gluster",
-  "iscsi")
+  "iscsi",
+  "http")
  
  VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,

"tcp",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 500a5be..77fa00c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -540,6 +540,7 @@ enum virDomainDiskProtocol {
  VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
  VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
  VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
+VIR_DOMAIN_DISK_PROTOCOL_HTTP,
  
  VIR_DOMAIN_DISK_PROTOCOL_LAST

  };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f151173..a7c6c8e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3826,6 +3826,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
  virBufferEscape(&opt, ',', ",", "%s,", disk->src);
  }
  break;
+
+case VIR_DOMAIN_DISK_PROTOCOL_HTTP: {
+virBufferAsprintf(&opt, "file=http://%s:%s";,
+  disk->hosts->name,
+  disk->hosts->port ? disk->hosts->port : 
"80");
+virBufferEscape(&opt, ',', ",", "%s,", disk->src);
+}

No need for {} around case blocks which don't have local variables
defined. As Eric mentions, missing 'break' here.


While you're doing this, how about also adding 'ftp' protocol support ?


Sure! I will do a patch for it too.




  }
  } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
  if (qemuBuildVolumeString(conn, disk, &opt) < 0)

Daniel


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

Re: [libvirt] pvpanic plans?

[Laszlo Ersek]

On 08/22/13 18:44, Anthony Liguori wrote:

> pvpanic has been a failure.  It's a poorly designed device with even
> worse semantics.

I disagree somewhat.

Requiring a separate ioport is not ideal, I admit. Configuration over
ACPI is good OTOH (it seems to put standards to good use anyway).

Noone realized pvpanic had poor technical design until the Windows "new
device" wizard popped up -- is that correct? Most of us are probably not
habitual Windows users, which is probably why we haven't thought of it
earlier.

Maybe we shouldn't promise "there won't be guest-visible changes in ACPI
contents". If we do promise, maybe we should then make the SeaBIOS
binary that we're loading dependent on -M too too.

After all, had we managed to completely hide the \_SB.PCI0.ISA.PEVT
device programmatically, as opposed to only disabling it, we might have
never realized pvpanic had poor design. Which (almost) means it wouldn't
have had one.

If we selected a SeaBIOS binary based on -M, then we could hide this
stuff from Windows.


> I applied it and I'll take the fault for merging it in
> the first place.
> 
> We should simply scrap it and start over.

That will kinda Eff some downstreams in the A...


> It has so few users at this
> point that this is still a realistic option.  Using something based on
> ISA that requires specific ACPI entries was a mistake.
> 
> We should just introduce a simple watchdog device based on virtio and
> call it a day.  Then it's cross platform, solves the guest enumeration
> problem, and libvirt can detect the presence of the new device.

If the guest doesn't initialize the proposed virtio-panic device, then
it will lie dormant too, just like the current pvpanic device. That's good.

However a new (standalone) virtio device will take up yet another PCI
function (a full device if you want it to be hotpluggable). PCI
functions are scarcer than ioports.

It will need documentation in the virtio-spec as well.

We'd need an arbitrarily heavily multiplexed paravirt channel between
guest and qemu. Maybe a dedicated virtio-serial port that's not exposed
to other host processes; one that qemu would "consume" itself.

If you want to be able to panic in boot firmware, writing to an ioport
is easier than adding a new virtio driver (virtio-serial, or a
completely new device).

> None of the plans outlined below give us a proper solution.  I think
> removing is our best option at this point.

I'm just trolling ^W playing the devil's advocate here, giving you more
opportunity to argue your point :)

Thanks,
Laszlo

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

Re: [libvirt] pvpanic plans?

[Laszlo Ersek]

On 08/22/13 18:10, Paolo Bonzini wrote:
> The thread from yesterday has died off (perhaps also because of
> my inappropriate answer to Michael, for which I apologize to him
> and everyone).  I took some time to discuss the libvirt requirements
> further with Daniel Berrange and Eric Blake on IRC.  If anyone is
> interested, I can give logs.  This is a suggestion for how to
> proceed in both QEMU and libvirt.

The analysis is pretty overwhelming :)

I have read Anthony's response and I'm not trying to argue -- I'm just
spending a few thoughts on this and I'm willing to let them go to waste.

In general I think we should minimize the quirks the user (who edits the
libvirt XML) has to know about. Interactions between some XML elements,
without explicit inter-references (formulated as attributes, like
controller/index) are Bad (TM). So,

> == Builtin pvpanic ==
> 
> QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
> break migration.
> 
> 
> == Support in libvirt for current functionality ==
> 
> libvirt will add a  element, and possibly a capability
> for it accessible via "virsh capabilities".  There are two possibilities:
> 
> 1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
>other than pc-1.5),  will only work if the element is there.
>On QEMU 1.5.0->1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
> will be obeyed always, and may override e.g. reboot-on-panic
>if a guest driver exist.

I don't like this because there's some interplay between on_crash and
panic_notifier, which even depends on the qemu version.


> 
> 2) On all versions,  will only work if the element is there.

I like this, because, if on_crash doesn't work without panic_notifier
*at all*, then we can just drop panic_notifier, and make on_crash mean
(on_crash && panic_notifier) in the original sense.

IOW, drop "panic_notifier", and make "on_crash" work *always*.

> 
> 
> In turn, there are two ways to implement (2):
> 
> 2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
> the builtin pvpanic device if present.  
> will create the device with -device pvpanic,iobase=0x505
> 
> Advantage: no changes to QEMU
> 
> Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
>   and pc-1.5 machine type will write to a pvpanic device instead of
>   the DMA controller.  Probably harmless, and limited to some QEMU
>   versions.
> 
> Disadvantage 2: libvirt has knowledge of the pvpanic port number

Updating this paragraph with my above suggestion:

- (s/pvpanic.iobase/pvpanic.ioport/g)

- if "on_crash" is absent:
  - for 1.{5.0,5.1,5.2,5.3,6.0}, add -global pvpanic.ioport=0
  - for other versions, do nothing

- if "on_crash" is present:
  - for 1.{5.0,5.1,5.2,5.3,6.0}, do nothing,
  - for other versions, pass -device pvpanic
(knowledge of 0x505 is unneeded)

"advantage" and "disadvantage 1" remain, "disadvantage 2" is gone.


> 2b) QEMU will provide a way for libvirt to detect that no machine type
> has the builtin pvpanic.  If some machine type may have the builtin
> pvpanic, and  is absent, libvirt will add
> "-global pvpanic.iobase=0" to neutralize it.  Otherwise, libvirt
> will create the device normally.
> 
> A possible way for libvirt to detect "good" machine types is a
> dummy property.  This is a bit ugly in that the property would not
> affect the behavior of the device.  The property would remain in
> the long term.
> 
> Another possibility is for QEMU to rename the device, e.g. to
> isa-pvpanic.  This is also somewhat gross, but not visible in the
> long term when the "pvpanic" name will be lost in history.
> 
> Advantage 1: libvirt has no knowledge of the pvpanic port number
> 
> Disadvantage 1: same as above
> 
> Disadvantage 2: need a somewhat gross change in QEMU
> 
> 
> This method also provides an (also somewhat gross on the QEMU side)
> way to detect other changes in the pvpanic semantics.  One example
> mentioned below, is making the panicked state temporary.

Too much work in qemu, in order to introduce ugliness, to hide older
ugliness.

> == Possible improvements to pvpanic ==

That's too complex / far out for me now, sorry :)

Thanks,
Laszlo

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

Re: [libvirt] [Qemu-devel] [PATCH 3/3] pvpanic: rename to isa-pvpanic

[Anthony Liguori]

Laszlo Ersek  writes:

> On 08/21/13 19:06, Paolo Bonzini wrote:
>> Il 21/08/2013 19:07, Michael S. Tsirkin ha scritto:
>
>>> NACK
>> 
>> You know that a single developer's NACK counts nothing (it can be you,
>> it can be me), don't you?
>
> going meta...
>
> What's this?
>
> All I know (... I think I know) about patch acceptance is that Anthony
> prefers to have at least one R-b. As far as I've seen this is not a hard
> requirement (for example, maintainers sometimes send unreviewed patches
> in a pull request, and on occasion they are merged).

I look very poorly on anyone nacking anything.  I value constructive
feedback.

Nacking does not add any value to the conversation.  I admire the fact
that we've been able to maintain a very high level of conversation over
the years on qemu-devel and throwing around nacks just lowers the
overall tone.

If you can't think of anything better to say than NACK, don't even
bother sending the email in the first place.

Regards,

Anthony Liguori

>
> No words have been spent on NAKs yet (... since my subscription, that
> is). Is this stuff formalized somewhere?
>
> Sorry for wasting time...
>
> Thanks,
> Laszlo

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

Re: [libvirt] pvpanic plans?

[Anthony Liguori]

Paolo Bonzini  writes:

> The thread from yesterday has died off (perhaps also because of
> my inappropriate answer to Michael, for which I apologize to him
> and everyone).  I took some time to discuss the libvirt requirements
> further with Daniel Berrange and Eric Blake on IRC.  If anyone is
> interested, I can give logs.  This is a suggestion for how to
> proceed in both QEMU and libvirt.
>
>
> == Builtin pvpanic ==
>
> QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
> break migration.

pvpanic has been a failure.  It's a poorly designed device with even
worse semantics.  I applied it and I'll take the fault for merging it in
the first place.

We should simply scrap it and start over.  It has so few users at this
point that this is still a realistic option.  Using something based on
ISA that requires specific ACPI entries was a mistake.

We should just introduce a simple watchdog device based on virtio and
call it a day.  Then it's cross platform, solves the guest enumeration
problem, and libvirt can detect the presence of the new device.

None of the plans outlined below give us a proper solution.  I think
removing is our best option at this point.

Regards,

Anthony Liguori

>
>
> == Support in libvirt for current functionality ==
>
> libvirt will add a  element, and possibly a capability
> for it accessible via "virsh capabilities".  There are two possibilities:
>
> 1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
>other than pc-1.5),  will only work if the element is there.
>On QEMU 1.5.0->1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
> will be obeyed always, and may override e.g. reboot-on-panic
>if a guest driver exist.
>
> 2) On all versions,  will only work if the element is there.
>
>
> In turn, there are two ways to implement (2):
>
> 2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
> the builtin pvpanic device if present.  
> will create the device with -device pvpanic,iobase=0x505
>
> Advantage: no changes to QEMU
>
> Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
>   and pc-1.5 machine type will write to a pvpanic device instead of
>   the DMA controller.  Probably harmless, and limited to some QEMU
>   versions.
>
> Disadvantage 2: libvirt has knowledge of the pvpanic port number
>
> 2b) QEMU will provide a way for libvirt to detect that no machine type
> has the builtin pvpanic.  If some machine type may have the builtin
> pvpanic, and  is absent, libvirt will add
> "-global pvpanic.iobase=0" to neutralize it.  Otherwise, libvirt
> will create the device normally.
>
> A possible way for libvirt to detect "good" machine types is a
> dummy property.  This is a bit ugly in that the property would not
> affect the behavior of the device.  The property would remain in
> the long term.
>
> Another possibility is for QEMU to rename the device, e.g. to
> isa-pvpanic.  This is also somewhat gross, but not visible in the
> long term when the "pvpanic" name will be lost in history.
>
> Advantage 1: libvirt has no knowledge of the pvpanic port number
>
> Disadvantage 1: same as above
>
> Disadvantage 2: need a somewhat gross change in QEMU
>
>
> This method also provides an (also somewhat gross on the QEMU side)
> way to detect other changes in the pvpanic semantics.  One example
> mentioned below, is making the panicked state temporary.
>
> == Possible improvements to pvpanic ==
>
> The current implementation of pvpanic supports three modes: reset system
> on panic, destroy domain on panic, preserve domain with no possibility
> to resume it.  (Optionally a domain can be dumped too).
>
> Long term, the choice to include pvpanic should not be on the guest
> admin's shoulders, but rather in libosinfo.  Thus, it would be nice to
> have a fourth mode where the panic is logged but the guest otherwise
> keeps running.  This mode would let libosinfo add pvpanic by default
> without affecting the guest's behavior on panic.
>
> With this change, ignore will behave as follows
> for the three possibilities above:
>
> (1)  With QEMU 1.5.0 to 1.6.1,  will _not_ obey the setting,
>  never (even if no  is specified).
>
>  libvirt will have to pick a fallback action.
>
>advantage of destroy as fallback: it is the default (but
>   note that restart is the default for virt-install)
>
>advantage of preserve as fallback: lets the admin examine
>   the panic
>
>advantage of restart as fallback: maximum availability of
>   the VM, it is the default for virt-install
>
> (2a) With QEMU 1.5.0 to 1.6.1,  will _not_ obey the setting
>  if  is specified.  libvirt has _no way_ to learn
>  about this, so the capability would always be present with these
>  QEMU versions and libosinfo would always add  with
>  these versions.  Given the libosinfo scenar

Re: [libvirt] [Qemu-devel] [PATCH] kvm: warn if num cpus is greater than num recommended

[Andreas Färber]

Am 22.08.2013 18:12, schrieb Eduardo Habkost:
> 
> On 22/08/2013, at 12:39, Andrew Jones  wrote:
> 
>> The comment in kvm_max_vcpus() states that it's using the recommended
>> procedure from the kernel API documentation to get the max number
>> of vcpus that kvm supports. It is, but by always returning the
>> maximum number supported. The maximum number should only be used
>> for development purposes. qemu should check KVM_CAP_NR_VCPUS for
>> the recommended number of vcpus. This patch adds a warning if a user
>> specifies a number of cpus between the recommended and max.
>>
>> Signed-off-by: Andrew Jones 
> 
> CCing libvir-list. It is probably interesting for libvirt to expose or warn 
> about the recommended VCPU limit somehow, and in this case a simple warning 
> on stderr won't be enough.
> 
>> ---
>> kvm-all.c | 45 +++--
>> 1 file changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 716860f617455..9092e13ae60ea 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -1313,24 +1313,24 @@ static int kvm_irqchip_create(KVMState *s)
>> return 0;
>> }
>>
>> -static int kvm_max_vcpus(KVMState *s)
>> +/* Find number of supported CPUs using the recommended
>> + * procedure from the kernel API documentation to cope with
>> + * older kernels that may be missing capabilities.
>> + */
>> +static int kvm_recommended_vcpus(KVMState *s)
>> {
>> int ret;
>>
>> -/* Find number of supported CPUs using the recommended
>> - * procedure from the kernel API documentation to cope with
>> - * older kernels that may be missing capabilities.
>> - */
>> -ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
>> -if (ret) {
>> -return ret;
>> -}
>> ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
>> -if (ret) {
>> -return ret;
>> -}
>> +return (ret) ? ret : 4;
>> +}
>>
>> -return 4;
>> +static int kvm_max_vcpus(KVMState *s)
>> +{
>> +int ret;
>> +
>> +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
>> +return (ret) ? ret : kvm_recommended_vcpus(s);
>> }
>>
>> int kvm_init(void)
>> @@ -1383,12 +1383,21 @@ int kvm_init(void)
>> goto err;
>> }
>>
>> -max_vcpus = kvm_max_vcpus(s);
>> +max_vcpus = kvm_recommended_vcpus(s);
>> if (smp_cpus > max_vcpus) {
>> -ret = -EINVAL;
>> -fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus 
>> "
>> -"supported by KVM (%d)\n", smp_cpus, max_vcpus);
>> -goto err;
>> +fprintf(stderr,
>> +"Warning: Number of SMP cpus requested (%d) exceeds "
>> +"recommended cpus supported by KVM (%d)\n",
>> +smp_cpus, max_vcpus);
>> +
>> +max_vcpus = kvm_max_vcpus(s);
>> +if (smp_cpus > max_vcpus) {
>> +ret = -EINVAL;
>> +fprintf(stderr, "Number of SMP cpus requested (%d) exceeds "
>> +"max cpus supported by KVM (%d)\n",
>> +smp_cpus, max_vcpus);
>> +goto err;
>> +}

Should at least the fatal one use the new error_report()?

>> }
>>
>> s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);

I notice that only checks in kvm_init() based on smp_cpus are touched
herein. Should we add similar checks to CPU hot-add code and thus
possibly move that into some per-vCPU code path?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

Re: [libvirt] [PATCH] kvm: warn if num cpus is greater than num recommended

[Eduardo Habkost]

On 22/08/2013, at 12:39, Andrew Jones  wrote:

> The comment in kvm_max_vcpus() states that it's using the recommended
> procedure from the kernel API documentation to get the max number
> of vcpus that kvm supports. It is, but by always returning the
> maximum number supported. The maximum number should only be used
> for development purposes. qemu should check KVM_CAP_NR_VCPUS for
> the recommended number of vcpus. This patch adds a warning if a user
> specifies a number of cpus between the recommended and max.
> 
> Signed-off-by: Andrew Jones 

CCing libvir-list. It is probably interesting for libvirt to expose or warn 
about the recommended VCPU limit somehow, and in this case a simple warning on 
stderr won't be enough.

> ---
> kvm-all.c | 45 +++--
> 1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 716860f617455..9092e13ae60ea 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1313,24 +1313,24 @@ static int kvm_irqchip_create(KVMState *s)
> return 0;
> }
> 
> -static int kvm_max_vcpus(KVMState *s)
> +/* Find number of supported CPUs using the recommended
> + * procedure from the kernel API documentation to cope with
> + * older kernels that may be missing capabilities.
> + */
> +static int kvm_recommended_vcpus(KVMState *s)
> {
> int ret;
> 
> -/* Find number of supported CPUs using the recommended
> - * procedure from the kernel API documentation to cope with
> - * older kernels that may be missing capabilities.
> - */
> -ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> -if (ret) {
> -return ret;
> -}
> ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> -if (ret) {
> -return ret;
> -}
> +return (ret) ? ret : 4;
> +}
> 
> -return 4;
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +int ret;
> +
> +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +return (ret) ? ret : kvm_recommended_vcpus(s);
> }
> 
> int kvm_init(void)
> @@ -1383,12 +1383,21 @@ int kvm_init(void)
> goto err;
> }
> 
> -max_vcpus = kvm_max_vcpus(s);
> +max_vcpus = kvm_recommended_vcpus(s);
> if (smp_cpus > max_vcpus) {
> -ret = -EINVAL;
> -fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus "
> -"supported by KVM (%d)\n", smp_cpus, max_vcpus);
> -goto err;
> +fprintf(stderr,
> +"Warning: Number of SMP cpus requested (%d) exceeds "
> +"recommended cpus supported by KVM (%d)\n",
> +smp_cpus, max_vcpus);
> +
> +max_vcpus = kvm_max_vcpus(s);
> +if (smp_cpus > max_vcpus) {
> +ret = -EINVAL;
> +fprintf(stderr, "Number of SMP cpus requested (%d) exceeds "
> +"max cpus supported by KVM (%d)\n",
> +smp_cpus, max_vcpus);
> +goto err;
> +}
> }
> 
> s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> -- 
> 1.8.1.4
> 

-- 
Eduardo 


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

[libvirt] pvpanic plans?

[Paolo Bonzini]

The thread from yesterday has died off (perhaps also because of
my inappropriate answer to Michael, for which I apologize to him
and everyone).  I took some time to discuss the libvirt requirements
further with Daniel Berrange and Eric Blake on IRC.  If anyone is
interested, I can give logs.  This is a suggestion for how to
proceed in both QEMU and libvirt.


== Builtin pvpanic ==

QEMU will remove pvpanic from pc-1.5 in 1.6.1 and 1.5.4.  This does not
break migration.


== Support in libvirt for current functionality ==

libvirt will add a  element, and possibly a capability
for it accessible via "virsh capabilities".  There are two possibilities:

1) On QEMU 1.5.4/1.6.1 and newer (and on QEMU 1.6.0 with a machine type
   other than pc-1.5),  will only work if the element is there.
   On QEMU 1.5.0->1.5.3, and on QEMU 1.6.0 with the pc-1.5 machine type,
will be obeyed always, and may override e.g. reboot-on-panic
   if a guest driver exist.

2) On all versions,  will only work if the element is there.


In turn, there are two ways to implement (2):

2a) libvirt will always add -global pvpanic.iobase=0 to neutralize
the builtin pvpanic device if present.  
will create the device with -device pvpanic,iobase=0x505

Advantage: no changes to QEMU

Disadvantage 1: writes to port 0 with QEMU 1.{5.0,5.1,5.2,5.3,6.0}
  and pc-1.5 machine type will write to a pvpanic device instead of
  the DMA controller.  Probably harmless, and limited to some QEMU
  versions.

Disadvantage 2: libvirt has knowledge of the pvpanic port number

2b) QEMU will provide a way for libvirt to detect that no machine type
has the builtin pvpanic.  If some machine type may have the builtin
pvpanic, and  is absent, libvirt will add
"-global pvpanic.iobase=0" to neutralize it.  Otherwise, libvirt
will create the device normally.

A possible way for libvirt to detect "good" machine types is a
dummy property.  This is a bit ugly in that the property would not
affect the behavior of the device.  The property would remain in
the long term.

Another possibility is for QEMU to rename the device, e.g. to
isa-pvpanic.  This is also somewhat gross, but not visible in the
long term when the "pvpanic" name will be lost in history.

Advantage 1: libvirt has no knowledge of the pvpanic port number

Disadvantage 1: same as above

Disadvantage 2: need a somewhat gross change in QEMU


This method also provides an (also somewhat gross on the QEMU side)
way to detect other changes in the pvpanic semantics.  One example
mentioned below, is making the panicked state temporary.

== Possible improvements to pvpanic ==

The current implementation of pvpanic supports three modes: reset system
on panic, destroy domain on panic, preserve domain with no possibility
to resume it.  (Optionally a domain can be dumped too).

Long term, the choice to include pvpanic should not be on the guest
admin's shoulders, but rather in libosinfo.  Thus, it would be nice to
have a fourth mode where the panic is logged but the guest otherwise
keeps running.  This mode would let libosinfo add pvpanic by default
without affecting the guest's behavior on panic.

With this change, ignore will behave as follows
for the three possibilities above:

(1)  With QEMU 1.5.0 to 1.6.1,  will _not_ obey the setting,
 never (even if no  is specified).

 libvirt will have to pick a fallback action.

   advantage of destroy as fallback: it is the default (but
  note that restart is the default for virt-install)

   advantage of preserve as fallback: lets the admin examine
  the panic

   advantage of restart as fallback: maximum availability of
  the VM, it is the default for virt-install

(2a) With QEMU 1.5.0 to 1.6.1,  will _not_ obey the setting
 if  is specified.  libvirt has _no way_ to learn
 about this, so the capability would always be present with these
 QEMU versions and libosinfo would always add  with
 these versions.  Given the libosinfo scenario being considered here,
 this is not very different from (1).

(2b) With QEMU 1.5.0 to 1.6.1, the  element will not
 be available and not exposed in libvirt capabilities.  Thus with
 this version libosinfo would omit  from the XML.
 Guest policy will always be followed correctly.


The problem in both (1) and (2a) can be summarized as follows.  First,
libvirt will have to implement and document a fallback action for buggy
QEMU.  Second, even though the problems would be limited to some version
of QEMU, they would be relatively hard to debug for a casual user, could
start happening randomly by updating any one of QEMU, libvirt, libosinfo
or the guest kernel, and there is no fallback action for libvirt that is
always correct.

Thus, considering future libosinfo support for pvpanic, (2b) is preferrable
in my opinion.

Now, making pvpanic reversible requires a change in QEMU (

Re: [libvirt] [PATCH] HACKING: Introduce basic Signed-off-by politic

[Viktor Mihajlovski]

On 08/22/2013 02:12 PM, Michal Privoznik wrote:

With the rising number of signed-off patches appearing on the list,
we should have policy what signed-off means, and advice (enforce?)
contributors to use it.

Signed-off-by: Michal Privoznik 
---
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 543c77e..f1a5c59 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -78,10 +78,14 @@
of the bug number is useful; but also summarize the issue
rather than making all readers follow the link.  You can use
'git shortlog -30' to get an idea of typical summary lines.
-  Libvirt does not currently attach any meaning to
-  Signed-off-by: lines, so it is up to you if you want to
-  include or omit them in the commit message.
-
+  Moreover, you should sign off your patches, meaning you are the
+  original author(s) of them and you have right to submit them under
+  the open source license indicated in the file. To add the
+  Signed-off-by: line to the commit message automatically,
+  you can tweak the git configuration: 

In fact there's also the case where you can have multiple sign-offs,
that of the original author and then of persons that have reworked the
original patch. I use that typically to indicate that I have been
rebasing, have edited the commit message or applied changes resulting 
from review comments when I send out patches originating from

a colleague. I.e. roughly similar to the subsystem maintainer policy
used by the kernel folks. So that should be allow too IMHO.

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCHv4 1/4] add hostdev passthrough common library

[Jim Fehlig]

Chunyan Liu wrote:
> Thanks very much! Still two places to confirm:
>
> 2013/8/21 Daniel P. Berrange  >
>
> On Mon, Aug 19, 2013 at 04:49:37PM -0400, cy...@suse.com
>  wrote:
> > diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> > new file mode 100644
> > index 000..1baa829
> > --- /dev/null
> > +++ b/src/util/virhostdev.c
> > +
> > +/* For virReportOOMError()  and virReportSystemError() */
>
> No need for this comment - this is standard practice for every source
> file
>
> > +#define VIR_FROM_THIS VIR_FROM_NONE
>
> > +
> > +/* Same name as in virDriver. For special check. */
> > +#define LIBXL_DRIVER_NAME "xenlight"
> > +#define QEMU_DRIVER_NAME  "QEMU"
>
> You're using this later to determine whether to use pci-back
> vs pci-stub. 
>
> I think it it would be preferrable to have the drivers pass
> in the name of their desired stub driver instead.
>
>
> I'm afraid there are some problems:
> Currently there are two places:
>  1. if   is not specified in pci hostdev .xml, use
> default stub driver. But to 'libxl' and 'qemu', the default stub
> driver is different (pciback vs pci-stub), so, need to check
> hypervisor driver name to decide default stub dirver name.
>  2. in detach-device, for 'qemu/kvm', it needs to check
> 'kvm_assigned_device', waiting for device cleanup. For 'libxl', it
> doesn't. So, need to check hypervisor driver name to add the extra
> processing.
>
> Besides, to 'qemu', not only the 'pci-stub' case, it could be
> 'pci-stub' or 'vfio'. To prepare domain hostdevs, just could not pass
> ONE stub driver name simply to virHostdev API (e.g,
> virHostdevPrepareDomainHostdevs).
>
> Any suggestions?

Perhaps these driver-specific items could be handled by callbacks into
the driver, similar to driver-specific XML parsing and formating
callbacks in the common domain_conf code.

>  
>
> > +static int
> > +virHostdevOnceInit(void)
> > +{
> > +char ebuf[1024];
> > +
> > +if (VIR_ALLOC(hostdevMgr) < 0)
> > +goto error;
> > +
> > +if ((hostdevMgr->activePciHostdevs = virPCIDeviceListNew())
> == NULL)
> > +goto error;
> > +
> > +if ((hostdevMgr->activeUsbHostdevs = virUSBDeviceListNew())
> == NULL)
> > +goto error;
> > +
> > +if ((hostdevMgr->inactivePciHostdevs =
> virPCIDeviceListNew()) == NULL)
> > +goto error;
> > +
> > +if ((hostdevMgr->activeScsiHostdevs =
> virSCSIDeviceListNew()) == NULL)
> > +goto error;
> > +
> > +if (virAsprintf(&hostdevMgr->stateDir, "%s",
> HOSTDEV_STATE_DIR) < 0)
> > +goto error;
> > +
> > +if (virFileMakePath(hostdevMgr->stateDir) < 0) {
> > +VIR_ERROR(_("Failed to create state dir '%s': %s"),
> > +  hostdevMgr->stateDir, virStrerror(errno,
> ebuf, sizeof(ebuf)));
>
> You should be using virReportError here
>
> > +goto error;
> > +}
> > +
> > +return 0;
> > +
> > +error:
>
> You should free the partially initialized 'hostdevMgr' instance & all
> its data
>
> > +return -1;
> > +}
> > +
> > +VIR_ONCE_GLOBAL_INIT(virHostdev)
> > +
> > +virHostdevManagerPtr
> > +virHostdevManagerGetDefault(void)
> > +{
> > +virHostdevInitialize();
>
> You should do
>
>if (virHostdevInitialize() < 0)
>return NULL;
>
> > +return hostdevMgr;
> > +}
> > +
>
>
>
> > +
> > +void
> > +virHostdevReAttachUsbHostdevs(virHostdevManagerPtr mgr,
> > +  const char *drv_name,
> > +  const char *dom_name,
> > +  virDomainHostdevDefPtr *hostdevs,
> > +  int nhostdevs)
> > +{
> > +size_t i;
> > +
> > +virObjectLock(mgr->activeUsbHostdevs);
>
> I wonder if we should get rid of the mutex locks in
> the PCI / USB device list objects, and instead have
> a single lock on the virHostdevManagerPtr instance.
>
> I noticed in qemu_hostdev.c, originally it used single driver lock,
> later changed to use pci/usb list object lock. Do you think a single
> lock is still preferred? If yes, I'll update.

I don't know the reason for that change, but it would seem desirable to
manage USB devices without needing to wait for some operation on a PCI
device to complete.

Regards,
Jim

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

Re: [libvirt] [PATCH v3] BSD: implement virNetDev(Set|Clear)IPv4Address

[Roman Bogorodskiy]

  Roman Bogorodskiy wrote:

>   Eric Blake wrote:
> 
> > On 08/11/2013 07:54 AM, Roman Bogorodskiy wrote:
> > > Provide an implementation of virNetDev(Set|Clear)IPv4Address based on
> > > BSD ifconfig tool in addition to 'ip' from Linux iproute2 package.
> > > ---
> > >  configure.ac | 15 +++
> > >  src/util/virnetdev.c | 24 
> > >  2 files changed, 39 insertions(+)
> > > 
> > 
> > >  
> > >  if test $with_freebsd = yes; then
> > > +   want_ifconfig=yes
> > > +
> > > with_firewalld=no
> > >  fi
> > >  
> > > @@ -2429,6 +2435,15 @@ AC_CHECK_DECLS([BRDGSFD, BRDGADD, BRDGDEL],
> > >  #include 
> > > ])
> > >  
> > > +# Check if we need to look for ifconfig
> > > +if test "$want_ifconfig" = "yes"; then
> > > + AC_PATH_PROG([IFCONFIG_PATH], [ifconfig])
> > > + if test -z "$IFCONFIG_PATH"; then
> > > + AC_MSG_ERROR([Failed to find ifconfig.])
> > 
> > This means that configure will fail if ifconfig is not installed but
> > want_ifconfig was set.  Are you certain enough that FreeBSD is likely to
> > have ifconfig by default, and thus not hit this failure except on
> > extremely unlikely configurations?
> 
> ifconfig is a part of FreeBSD base system. And appears it's not even
> possible to disable installing it via make.conf [1]. So I doubt there
> are a lot of systems without ifconfig, and even if they exist I'm not
> sure we need to support them because, as you said, it's extremely
> unlikely configuration.
> 
> 1: http://www.freebsd.org/cgi/man.cgi?query=make.conf&sektion=5

One correction:

The proper man would be:

http://www.freebsd.org/cgi/man.cgi?query=src.conf

It's possible to build using WITHOUT_INET to disable IPv4 networking,
but again, I doubt that we should support such systems.

Roman Bogorodskiy


pgp6UPCZ_8L28.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3] BSD: implement virNetDev(Set|Clear)IPv4Address

[Roman Bogorodskiy]

  Eric Blake wrote:

> On 08/11/2013 07:54 AM, Roman Bogorodskiy wrote:
> > Provide an implementation of virNetDev(Set|Clear)IPv4Address based on
> > BSD ifconfig tool in addition to 'ip' from Linux iproute2 package.
> > ---
> >  configure.ac | 15 +++
> >  src/util/virnetdev.c | 24 
> >  2 files changed, 39 insertions(+)
> > 
> 
> >  
> >  if test $with_freebsd = yes; then
> > +   want_ifconfig=yes
> > +
> > with_firewalld=no
> >  fi
> >  
> > @@ -2429,6 +2435,15 @@ AC_CHECK_DECLS([BRDGSFD, BRDGADD, BRDGDEL],
> >  #include 
> > ])
> >  
> > +# Check if we need to look for ifconfig
> > +if test "$want_ifconfig" = "yes"; then
> > + AC_PATH_PROG([IFCONFIG_PATH], [ifconfig])
> > + if test -z "$IFCONFIG_PATH"; then
> > + AC_MSG_ERROR([Failed to find ifconfig.])
> 
> This means that configure will fail if ifconfig is not installed but
> want_ifconfig was set.  Are you certain enough that FreeBSD is likely to
> have ifconfig by default, and thus not hit this failure except on
> extremely unlikely configurations?

ifconfig is a part of FreeBSD base system. And appears it's not even
possible to disable installing it via make.conf [1]. So I doubt there
are a lot of systems without ifconfig, and even if they exist I'm not
sure we need to support them because, as you said, it's extremely
unlikely configuration.

1: http://www.freebsd.org/cgi/man.cgi?query=make.conf&sektion=5

> But I think it looks simple enough, with minimal risk of causing grief
> on non-BSD systems (the configure check is guarded, so IFCONFIG_PATH is
> likely to be unset on other platforms).  ACK and pushed.
> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 



Roman Bogorodskiy


pgpTeRbxQqVDh.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking

[Eric Blake]

On 08/22/2013 07:32 AM, Martin Kletzander wrote:

> 
>> Everything looks portable - I have no objection to this patch as-is,
>> although if you haven't pushed yet, you can consider tweaking the nits I
>> pointed out.
>>
> 
> Unfortunately, I pushed it with the thought that 2 ACKs should cover
> anything.  However, I'll be more than happy to fix these in a follow-up
> patch if you think it's worth it.

No, nothing I pointed out was fatal, so I won't worry if there is no
followup.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking

[Martin Kletzander]

On Thu 22 Aug 2013 03:15:48 PM CEST, Eric Blake wrote:
> On 08/22/2013 06:19 AM, Martin Kletzander wrote:
>> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
>> to deal with this issue again, herec comes "virsh-uriprecedence".
>>
>> Signed-off-by: Martin Kletzander 
>> ---
>
>> @@ -235,6 +236,7 @@ test_scripts +=  \
>>  read-bufsiz \
>>  read-non-seekable   \
>>  start   \
>> +virsh-uriprecedence \
>>  vcpupin \
>
> No longer sorted after the rename :)
>

Oh, rushing the v2 always gets me :(

>> +++ b/tests/virsh-uriprecedence
>> @@ -0,0 +1,76 @@
>> +#!/bin/sh
>
> I'm reviewing this for POSIX sh compatibility...
>
>> +# Create all mock files/directories to avoid permission problems
>> +tmphome="$PWD/tmp_home"
>> +export XDG_CONFIG_HOME="$tmphome/.config"
>> +export XDG_CACHE_HOME="$tmphome/.cache"
>> +export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
>> +
>> +mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
>> +mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
>> +mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
>
> Isn't this last line redundant, since XDG_RUNTIME_HOME is equal to
> XDG_CACHE_HOME?  Also, you could use one 'mkdir -p' to make all the
> directories (but shaving processes isn't essential).
>

Yes, it can.  I thought it'll be better for the future and
understandability and won't hurt in case there's one 'mkdir -p'.
Unfortunately I the haven't used it.

>> +
>> +printf "uri_default=\"%s\"\n" "$good_uri" 
>> >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
>
> Style - slightly shorter as printf 'uri_default="%s"\n' ...
> Long line - is it worth wrapping with \-newline?
>

Definitely, I missed this one.

> Everything looks portable - I have no objection to this patch as-is,
> although if you haven't pushed yet, you can consider tweaking the nits I
> pointed out.
>

Unfortunately, I pushed it with the thought that 2 ACKs should cover
anything.  However, I'll be more than happy to fix these in a follow-up
patch if you think it's worth it.

Martin

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

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking

[Eric Blake]

On 08/22/2013 06:19 AM, Martin Kletzander wrote:
> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
> to deal with this issue again, herec comes "virsh-uriprecedence".
> 
> Signed-off-by: Martin Kletzander 
> ---

> @@ -235,6 +236,7 @@ test_scripts +=   \
>   read-bufsiz \
>   read-non-seekable   \
>   start   \
> + virsh-uriprecedence \
>   vcpupin \

No longer sorted after the rename :)

> +++ b/tests/virsh-uriprecedence
> @@ -0,0 +1,76 @@
> +#!/bin/sh

I'm reviewing this for POSIX sh compatibility...

> +# Create all mock files/directories to avoid permission problems
> +tmphome="$PWD/tmp_home"
> +export XDG_CONFIG_HOME="$tmphome/.config"
> +export XDG_CACHE_HOME="$tmphome/.cache"
> +export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
> +
> +mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
> +mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
> +mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"

Isn't this last line redundant, since XDG_RUNTIME_HOME is equal to
XDG_CACHE_HOME?  Also, you could use one 'mkdir -p' to make all the
directories (but shaving processes isn't essential).

> +
> +printf "uri_default=\"%s\"\n" "$good_uri" 
> >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"

Style - slightly shorter as printf 'uri_default="%s"\n' ...
Long line - is it worth wrapping with \-newline?

Everything looks portable - I have no objection to this patch as-is,
although if you haven't pushed yet, you can consider tweaking the nits I
pointed out.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking

[Martin Kletzander]

On 08/22/2013 02:56 PM, Daniel P. Berrange wrote:
> On Thu, Aug 22, 2013 at 02:19:56PM +0200, Martin Kletzander wrote:
>> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
>> to deal with this issue again, herec comes "virsh-uriprecedence".
>>
>> Signed-off-by: Martin Kletzander 
>> ---
>>
>> Notes:
>> v2:
>>  - Changed the name to virsh-uriprecedence
>>  - Using $PWD instead of temporary dir
>>
>>  tests/Makefile.am |  2 ++
>>  tests/virsh-uriprecedence | 76 
>> +++
>>  2 files changed, 78 insertions(+)
>>  create mode 100755 tests/virsh-uriprecedence
> 
> ACK
> 

Thanks guys, pushed now.

Martin

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

Re: [libvirt] [PATCH] Allow a HTTP URL for cdrom ISO image

[Daniel P. Berrange]

On Wed, Aug 21, 2013 at 04:31:03PM -0300, Aline Manera wrote:
> From: Aline Manera 
> 
> QEMU/KVM already allows an HTTP URL for the cdrom ISO image so add this 
> support
> to libvirt as well.
> The xml should be as following:
> 
> 
>   
> 
>   
> 
> 
> Signed-off-by: Aline Manera 
> ---
>  src/conf/domain_conf.c |3 +-
>  src/conf/domain_conf.h |1 +
>  src/qemu/qemu_command.c|7 
>  .../qemuxml2argv-disk-cdrom-network.args   |5 +++
>  .../qemuxml2argv-disk-cdrom-network.xml|   37 
> 
>  tests/qemuxml2argvtest.c   |2 ++
>  6 files changed, 54 insertions(+), 1 deletion(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ea49d2c..461cc95 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -261,7 +261,8 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, 
> VIR_DOMAIN_DISK_PROTOCOL_LAST,
>"rbd",
>"sheepdog",
>"gluster",
> -  "iscsi")
> +  "iscsi",
> +  "http")
>  
>  VIR_ENUM_IMPL(virDomainDiskProtocolTransport, 
> VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
>"tcp",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 500a5be..77fa00c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -540,6 +540,7 @@ enum virDomainDiskProtocol {
>  VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
>  VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
>  VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
> +VIR_DOMAIN_DISK_PROTOCOL_HTTP,
>  
>  VIR_DOMAIN_DISK_PROTOCOL_LAST
>  };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f151173..a7c6c8e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3826,6 +3826,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>  virBufferEscape(&opt, ',', ",", "%s,", disk->src);
>  }
>  break;
> +
> +case VIR_DOMAIN_DISK_PROTOCOL_HTTP: {
> +virBufferAsprintf(&opt, "file=http://%s:%s";,
> +  disk->hosts->name,
> +  disk->hosts->port ? disk->hosts->port : 
> "80");
> +virBufferEscape(&opt, ',', ",", "%s,", disk->src);
> +}

No need for {} around case blocks which don't have local variables
defined. As Eric mentions, missing 'break' here.


While you're doing this, how about also adding 'ftp' protocol support ?

>  }
>  } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
>  if (qemuBuildVolumeString(conn, disk, &opt) < 0)

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

Re: [libvirt] [PATCH] HACKING: Introduce basic Signed-off-by politic

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 02:12:07PM +0200, Michal Privoznik wrote:
> With the rising number of signed-off patches appearing on the list,
> we should have policy what signed-off means, and advice (enforce?)
> contributors to use it.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  HACKING  |  8 ++--
>  docs/hacking.html.in | 12 
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/HACKING b/HACKING
> index f9f8381..e54e584 100644
> --- a/HACKING
> +++ b/HACKING
> @@ -73,8 +73,12 @@ commit introduced the problem, mentioning that is useful. 
> If the patch
>  resolves a bugzilla report, mentioning the URL of the bug number is useful;
>  but also summarize the issue rather than making all readers follow the link.
>  You can use 'git shortlog -30' to get an idea of typical summary lines.
> -Libvirt does not currently attach any meaning to Signed-off-by: lines, so it
> -is up to you if you want to include or omit them in the commit message.
> +Moreover, you should sign off your patches, meaning you are the original
> +author(s) of them and you have right to submit them under the open source
> +license indicated in the file. To add the "Signed-off-by:" line to the commit
> +message automatically, you can tweak the git configuration:
> +
> +  git config format.signoff true

I think if we're going todo this, we should be a bit more verbose. IOW,
I'd suggest right near the start of the hacking file we basically copy
the kernel's text on this matter


[quote]
To improve tracking of who did what, especially with patches that can
percolate to their final resting place in the kernel through several
layers of maintainers, we've introduced a "sign-off" procedure on
patches that are being emailed around.

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.  The rules are pretty simple: if you
can certify the below:

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

then you just add a line saying

Signed-off-by: Random J Developer 

using your real name (sorry, no pseudonyms or anonymous contributions.)
[/quote]


The kernel's docs also describe  use of Acked-by, Tested-By and
many other annotations, if we want to really take this approach
fully.

Regards,
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

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 02:19:56PM +0200, Martin Kletzander wrote:
> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
> to deal with this issue again, herec comes "virsh-uriprecedence".
> 
> Signed-off-by: Martin Kletzander 
> ---
> 
> Notes:
> v2:
>  - Changed the name to virsh-uriprecedence
>  - Using $PWD instead of temporary dir
> 
>  tests/Makefile.am |  2 ++
>  tests/virsh-uriprecedence | 76 
> +++
>  2 files changed, 78 insertions(+)
>  create mode 100755 tests/virsh-uriprecedence

ACK


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

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking

[Ján Tomko]

On 08/22/2013 02:19 PM, Martin Kletzander wrote:
> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
> to deal with this issue again, herec comes "virsh-uriprecedence".
> 
> Signed-off-by: Martin Kletzander 
> ---
> 
> Notes:
> v2:
>  - Changed the name to virsh-uriprecedence
>  - Using $PWD instead of temporary dir
> 
>  tests/Makefile.am |  2 ++
>  tests/virsh-uriprecedence | 76 
> +++
>  2 files changed, 78 insertions(+)
>  create mode 100755 tests/virsh-uriprecedence

ACK

Jan

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

Re: [libvirt] [Qemu-devel] [PATCH 3/3] pvpanic: rename to isa-pvpanic

[Paolo Bonzini]

Il 22/08/2013 14:43, Laszlo Ersek ha scritto:
> On 08/21/13 19:06, Paolo Bonzini wrote:
>> Il 21/08/2013 19:07, Michael S. Tsirkin ha scritto:
> 
>>> NACK
>>
>> You know that a single developer's NACK counts nothing (it can be you,
>> it can be me), don't you?
> 
> going meta...
> 
> What's this?
> 
> All I know (... I think I know) about patch acceptance is that Anthony
> prefers to have at least one R-b. As far as I've seen this is not a hard
> requirement (for example, maintainers sometimes send unreviewed patches
> in a pull request, and on occasion they are merged).
> 
> No words have been spent on NAKs yet (... since my subscription, that
> is). Is this stuff formalized somewhere?
> 
> Sorry for wasting time...

No, it's not.  But for example I NACKed removal of pvpanic from 1.6, it
was overridden, and I didn't complain too much.

Paolo

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

Re: [libvirt] [Qemu-devel] [PATCH 3/3] pvpanic: rename to isa-pvpanic

[Laszlo Ersek]

On 08/21/13 19:06, Paolo Bonzini wrote:
> Il 21/08/2013 19:07, Michael S. Tsirkin ha scritto:

>> NACK
> 
> You know that a single developer's NACK counts nothing (it can be you,
> it can be me), don't you?

going meta...

What's this?

All I know (... I think I know) about patch acceptance is that Anthony
prefers to have at least one R-b. As far as I've seen this is not a hard
requirement (for example, maintainers sometimes send unreviewed patches
in a pull request, and on occasion they are merged).

No words have been spent on NAKs yet (... since my subscription, that
is). Is this stuff formalized somewhere?

Sorry for wasting time...

Thanks,
Laszlo

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

[libvirt] [PATCH v2] tests: Add URI precedence checking

[Martin Kletzander]

Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
to deal with this issue again, herec comes "virsh-uriprecedence".

Signed-off-by: Martin Kletzander 
---

Notes:
v2:
 - Changed the name to virsh-uriprecedence
 - Using $PWD instead of temporary dir

 tests/Makefile.am |  2 ++
 tests/virsh-uriprecedence | 76 +++
 2 files changed, 78 insertions(+)
 create mode 100755 tests/virsh-uriprecedence

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 09144d6..2fb8f37 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -98,6 +98,7 @@ EXTRA_DIST =  \
storagevolxml2xmlout \
sysinfodata \
test-lib.sh \
+   virsh-uriprecedence \
vmx2xmldata \
xencapsdata \
xmconfigdata \
@@ -235,6 +236,7 @@ test_scripts += \
read-bufsiz \
read-non-seekable   \
start   \
+   virsh-uriprecedence \
vcpupin \
virsh-all   \
virsh-optparse  \
diff --git a/tests/virsh-uriprecedence b/tests/virsh-uriprecedence
new file mode 100755
index 000..f4d84a4
--- /dev/null
+++ b/tests/virsh-uriprecedence
@@ -0,0 +1,76 @@
+#!/bin/sh
+
+: ${srcdir=.}
+. $srcdir/test-lib.sh
+
+# This test checks if virsh obeys the proper precedence of different
+# URI settings
+test_intro "virsh-uriprecedence"
+
+virsh_bin="$abs_top_builddir/tools/virsh"
+counter=1
+ret=0
+
+cleanup_() { rm -rf "$tmphome"; }
+
+# Create all mock files/directories to avoid permission problems
+tmphome="$PWD/tmp_home"
+export XDG_CONFIG_HOME="$tmphome/.config"
+export XDG_CACHE_HOME="$tmphome/.cache"
+export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
+
+mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
+mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
+mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
+
+# Main function checking for the proper uri being returned
+test_uri()
+{
+  result=0
+  if [ "$($virsh_bin uri)" != "$good_uri" ]; then
+  result=1
+  ret=1
+  fi
+  test_result "$counter" "$1" "$result"
+  counter="$((counter+1))"
+}
+
+# Precedence is the following (lowest priority first):
+#
+# 1) if run as root, 'uri_default' from /etc/libvirtd/libvirt.conf,
+#otherwise qemu:///session.  There is no way to mock this file for
+#virsh/libvirt.so and the user may have set anything in there that
+#would spoil the test, so we don't test this
+#
+# 2) 'uri_default' from $XDG_CONFIG_HOME/libvirt/libvirt.conf
+#
+# 3) LIBVIRT_DEFAULT_URI
+#
+# 4) VIRSH_DEFAULT_CONNECT_URI
+#
+# 5) parameter -c (--connect)
+
+unset LIBVIRT_DEFAULT_URI
+unset VIRSH_DEFAULT_CONNECT_URI
+bad_uri="test:///default?bad_uri"
+good_uri="test:///default?good_uri"
+
+printf "uri_default=\"%s\"\n" "$good_uri" 
>"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
+test_uri "User config file"
+
+printf "uri_default=\"%s\"\n" "$bad_uri" 
>"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
+export LIBVIRT_DEFAULT_URI="$good_uri"
+test_uri "LIBVIRT_DEFAULT_URI"
+
+export LIBVIRT_DEFAULT_URI="$bad_uri"
+export VIRSH_DEFAULT_CONNECT_URI="$good_uri"
+test_uri "VIRSH_DEFAULT_CONNECT_URI"
+
+export VIRSH_DEFAULT_CONNECT_URI="$bad_uri"
+virsh_bin="$virsh_bin --connect $good_uri"
+test_uri "Parameter"
+
+# test_uri() increases $counter even for the last test, so we must
+# decrement it
+test_final "$((counter-1))" "$ret"
+(exit "$ret"); exit "$ret"
-- 
1.8.3.2

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

[libvirt] [PATCH] HACKING: Introduce basic Signed-off-by politic

[Michal Privoznik]

With the rising number of signed-off patches appearing on the list,
we should have policy what signed-off means, and advice (enforce?)
contributors to use it.

Signed-off-by: Michal Privoznik 
---
 HACKING  |  8 ++--
 docs/hacking.html.in | 12 
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/HACKING b/HACKING
index f9f8381..e54e584 100644
--- a/HACKING
+++ b/HACKING
@@ -73,8 +73,12 @@ commit introduced the problem, mentioning that is useful. If 
the patch
 resolves a bugzilla report, mentioning the URL of the bug number is useful;
 but also summarize the issue rather than making all readers follow the link.
 You can use 'git shortlog -30' to get an idea of typical summary lines.
-Libvirt does not currently attach any meaning to Signed-off-by: lines, so it
-is up to you if you want to include or omit them in the commit message.
+Moreover, you should sign off your patches, meaning you are the original
+author(s) of them and you have right to submit them under the open source
+license indicated in the file. To add the "Signed-off-by:" line to the commit
+message automatically, you can tweak the git configuration:
+
+  git config format.signoff true
 
 
 
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 543c77e..f1a5c59 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -78,10 +78,14 @@
   of the bug number is useful; but also summarize the issue
   rather than making all readers follow the link.  You can use
   'git shortlog -30' to get an idea of typical summary lines.
-  Libvirt does not currently attach any meaning to
-  Signed-off-by: lines, so it is up to you if you want to
-  include or omit them in the commit message.
-
+  Moreover, you should sign off your patches, meaning you are the
+  original author(s) of them and you have right to submit them under
+  the open source license indicated in the file. To add the
+  Signed-off-by: line to the commit message automatically,
+  you can tweak the git configuration: 
+
+  git config format.signoff true
+
   
 
   Split large changes into a series of smaller patches,
-- 
1.8.1.5

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

Re: [libvirt] [PATCH] Set security label on FD for virDomainOpenGraphics

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 01:52:17PM +0200, Michal Privoznik wrote:
> On 22.08.2013 13:39, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" 
> > 
> > The virDomainOpenGraphics method accepts a UNIX socket FD from
> > the client app. It must set the label on this FD otherwise QEMU
> > will be prevented from receiving it with recvmsg.
> > 
> > Signed-off-by: Daniel P. Berrange 
> 
> (*)

> * Side note - I've noticed more and more signed-off patches. Does this
> mean we are seamlessly moving to make it a standard?

I do it as a matter of habit on everything I commit these days to any
project I'm involved in.

If people think we should make it mandatory for libvirt, I'd be supportive
of that, but I don't feel strongly enough to force the issue myself right
now.

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

Re: [libvirt] [PATCH] Set security label on FD for virDomainOpenGraphics

[Michal Privoznik]

On 22.08.2013 13:39, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" 
> 
> The virDomainOpenGraphics method accepts a UNIX socket FD from
> the client app. It must set the label on this FD otherwise QEMU
> will be prevented from receiving it with recvmsg.
> 
> Signed-off-by: Daniel P. Berrange 

(*)

> ---
>  src/qemu/qemu_driver.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5124f27..0a8e518 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14777,6 +14777,10 @@ qemuDomainOpenGraphics(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> +if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
> +  fd) < 0)
> +goto cleanup;
> +
>  if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>  goto cleanup;
>  qemuDomainObjEnterMonitor(driver, vm);
> 

ACK

Michal

* Side note - I've noticed more and more signed-off patches. Does this
mean we are seamlessly moving to make it a standard?

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

Re: [libvirt] [PATCH v2 4/9] virsh: Add vshDomainCompleter

[Michal Privoznik]

On 20.08.2013 22:02, Tomas Meszaros wrote:
> * global variable __my_conn renamed to vshConn
> * @name is now const char *
> * label cleanup renamed to error
> ---
>  tools/virsh.c | 53 +
>  tools/virsh.h |  2 ++
>  2 files changed, 55 insertions(+)


You can be adding .copleter = vshDomainCompleter in this patch as well.
That is squash parts of 7/9, 8/9 and 9/9 which add the
vshDomainCompleter into this patch. In the end, They will probably end
up being empty, but that is okay.


> diff --git a/tools/virsh.c b/tools/virsh.c
> index af6e939..13c27df 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -88,6 +88,8 @@ static char *progname;
>  
>  static const vshCmdGrp cmdGroups[];
>  
> +virConnectPtr *vshConn;
> +
>  /* Bypass header poison */
>  #undef strdup
>  
> @@ -2503,6 +2505,51 @@ vshCloseLogFile(vshControl *ctl)
>  
>  #ifdef USE_READLINE
>  
> +/* -
> + * Completers
> + * -
> + */
> +
> +char **
> +vshDomainCompleter(unsigned int flags)
> +{
> +virDomainPtr *domains;
> +size_t i;
> +char **names = NULL;
> +int ndomains;
> +
> +if (!*vshConn)
> +return NULL;
> +
> +ndomains = virConnectListAllDomains(*vshConn, &domains, flags);
> +
> +if (ndomains < 0)
> +return NULL;
> +
> +names = vshMalloc(NULL, sizeof(char *) * (ndomains + 1));
> +
> +if (!names)
> +return NULL;

Well, we should never get to this situation. vshMalloc either fails and
exit() or return a valid pointer.

> +
> +for (i = 0; i < ndomains; i++) {
> +const char *name = virDomainGetName(domains[i]);
> +if (VIR_STRDUP(names[i], name) < 0) {
> +virDomainFree(domains[i]);
> +goto error;
> +}
> +virDomainFree(domains[i]);
> +}
> +names[i] = NULL;
> +VIR_FREE(domains);
> +return names;
> +
> +error:
> +for (i = 0; names[i]; i++)
> +VIR_FREE(names[i]);
> +VIR_FREE(names);
> +return NULL;
> +}
> +
>  /* -
>   * Readline stuff
>   * -
> @@ -3510,6 +3557,7 @@ main(int argc, char **argv)
>  ctl->debug = VSH_DEBUG_DEFAULT;
>  ctl->escapeChar = "^]"; /* Same default as telnet */
>  
> +vshConn = &ctl->conn;
>  
>  if (!setlocale(LC_ALL, "")) {
>  perror("setlocale");
> @@ -3571,6 +3619,11 @@ main(int argc, char **argv)
>  exit(EXIT_FAILURE);
>  }
>  
> +/* Need to connect immediately after start in order to provide
> + * autocompletion for the very first command.
> + */
> +vshReconnect(ctl);
> +

This has been discussed so see Eric's and Peter's replies.

>  do {
>  const char *prompt = ctl->readonly ? VSH_PROMPT_RO : 
> VSH_PROMPT_RW;
>  ctl->cmdstr =
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 064acde..68414e4 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -255,6 +255,8 @@ struct _vshCmdGrp {
>  const vshCmdDef *commands;
>  };
>  
> +char **vshDomainCompleter(unsigned int flags);
> +
>  void vshError(vshControl *ctl, const char *format, ...)
>  ATTRIBUTE_FMT_PRINTF(2, 3);
>  void vshOpenLogFile(vshControl *ctl);
> 

Michal

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

Re: [libvirt] [PATCH v2 6/9] virsh: Add vshRebootShutdownModeCompleter

[Michal Privoznik]

On 20.08.2013 22:02, Tomas Meszaros wrote:
> vshRebootShutdownModeCompleter returns available modes
> for reboot/shutdown commands.
> ---
>  tools/virsh.c | 28 
>  tools/virsh.h |  1 +
>  2 files changed, 29 insertions(+)
> 

Again, you should add .completer = vshRebootShutdownModeCompleter in
this patch.

> diff --git a/tools/virsh.c b/tools/virsh.c
> index 85d74ad..965b141 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2578,6 +2578,34 @@ error:
>  return NULL;
>  }
>  
> +char **
> +vshRebootShutdownModeCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
> +{
> +const char *modes[] = {"acpi", "agent", "initctl", "signal"};
> +const unsigned int modes_size = ARRAY_CARDINALITY(modes);
> +char **names = NULL;
> +size_t i;
> +
> +names = vshMalloc(NULL, sizeof(char *) * (modes_size + 1));
> +
> +if (!names)
> +return NULL;

Again useless if.

> +
> +for (i = 0; i < modes_size; i++) {
> +if (VIR_STRDUP(names[i], modes[i]) < 0)
> +goto cleanup;
> +}
> +
> +names[i] = NULL;
> +return names;
> +
> +cleanup:
> +for (i = 0; names[i]; i++)
> +VIR_FREE(names[i]);
> +VIR_FREE(names);
> +return NULL;
> +}
> +
>  /* -
>   * Readline stuff
>   * -
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 6767e65..61510b0 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -257,6 +257,7 @@ struct _vshCmdGrp {
>  
>  char **vshDomainCompleter(unsigned int flags);
>  char **vshSuspendTargetCompleter(unsigned int unused_flags);
> +char **vshRebootShutdownModeCompleter(unsigned int unused_flags);
>  
>  void vshError(vshControl *ctl, const char *format, ...)
>  ATTRIBUTE_FMT_PRINTF(2, 3);
> 

Michal

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

Re: [libvirt] [PATCH v2 5/9] virsh: Add vshSuspendTargetCompleter

[Michal Privoznik]

On 20.08.2013 22:02, Tomas Meszaros wrote:
> * label cleanup renamed to error
> * vshSuspendTargetCompleter added to opts_dom_pm_suspend
> ---
>  tools/virsh-domain.c |  3 ++-
>  tools/virsh.c| 28 
>  tools/virsh.h|  1 +
>  3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 5d4913d..a2002c5 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2756,7 +2756,8 @@ static const vshCmdOptDef opts_dom_pm_suspend[] = {
>   .flags = VSH_OFLAG_REQ,
>   .help = N_("mem(Suspend-to-RAM), "
>  "disk(Suspend-to-Disk), "
> -"hybrid(Hybrid-Suspend)")
> +"hybrid(Hybrid-Suspend)"),
> + .completer = vshSuspendTargetCompleter

This is what I had on my mind in 4/9.

>  },
>  {.name = NULL}
>  };
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 13c27df..85d74ad 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2550,6 +2550,34 @@ error:
>  return NULL;
>  }
>  
> +char **
> +vshSuspendTargetCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
> +{
> +const char *targets[] = {"mem", "disk", "hybrid"};
> +const unsigned int targets_size = ARRAY_CARDINALITY(targets);
> +char **names = NULL;
> +size_t i;
> +
> +names = vshMalloc(NULL, sizeof(char *) * (targets_size + 1));
> +
> +if (!names)
> +return NULL;
> +
> +for (i = 0; i < targets_size; i++) {
> +if (VIR_STRDUP(names[i], targets[i]) < 0)
> +goto error;
> +}
> +
> +names[i] = NULL;
> +return names;
> +
> +error:
> +for (i = 0; names[i]; i++)
> +VIR_FREE(names[i]);
> +VIR_FREE(names);
> +return NULL;
> +}
> +
>  /* -
>   * Readline stuff
>   * -
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 68414e4..6767e65 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -256,6 +256,7 @@ struct _vshCmdGrp {
>  };
>  
>  char **vshDomainCompleter(unsigned int flags);
> +char **vshSuspendTargetCompleter(unsigned int unused_flags);
>  
>  void vshError(vshControl *ctl, const char *format, ...)
>  ATTRIBUTE_FMT_PRINTF(2, 3);
> 

Michal

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

Re: [libvirt] [PATCH v2 3/9] virsh: Improve readline generators and readline completion

[Michal Privoznik]

On 20.08.2013 22:02, Tomas Meszaros wrote:
> * vshMalloc is now used in vshDetermineCommandName
> * vshStrdup instead of vshMalloc + snprintf
> * joined if's in vshReadlineOptionsCompletionGenerator
> ---
>  tools/virsh.c | 395 
> ++
>  1 file changed, 373 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 15f529e..af6e939 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c

> @@ -2605,22 +2613,365 @@ vshReadlineOptionsGenerator(const char *text, int 
> state)
>  return NULL;
>  }
>  
> +/*
> + * Generator function for command completion, but unlike
> + * the vshRaadlineCommandGenerator which completes command name, this 
> function

s/vshRaadlineCommandGenerator/tools/vshReadlineCommandGenerator/  (Raad
vs Read).

> + * provides more advanced completion for commands by calling specific command
> + * completers (e.g. vshDomainCompleter).
> + */
> +static char *
> +vshReadlineCommandCompletionGenerator(const char *text, int state)
> +{
> +static const vshCmdDef *cmd = NULL;

> +/*
> + * Returns current valid command option name present in the rl_line_buffer.
> + */
> +static char *
> +vshCurrentOpt(const char *cmd_name)
> +{
> +const vshCmdDef *cmd = NULL;
> +const char *name;
> +unsigned long int opt_index = 0;
> +size_t cmd_opt_list_index;
> +char *found_opt = NULL;
> +char *ptr = NULL;
> +
> +if (!cmd_name)
> +return NULL;
> +
> +cmd = vshCmddefSearch(cmd_name);
> +
> +if (!cmd)
> +return NULL;
> +
> +if (!cmd->opts)
> +return NULL;
> +
> +cmd_opt_list_index = 0;
> +
> +while ((name = cmd->opts[cmd_opt_list_index].name)) {
> +cmd_opt_list_index++;
> +
> +if ((ptr = strstr(rl_line_buffer, name))) {
> +if (opt_index < (ptr - rl_line_buffer)) {
> +opt_index = ptr - rl_line_buffer;
> +if (VIR_STRDUP(found_opt, name) < 0)
> +return NULL;
> +}
> +}
> +}
> +
> +if (!found_opt)
> +return NULL;

this if is useless.

> +
> +return found_opt;
> +}
> +

Michal

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

[libvirt] [PATCH] Set security label on FD for virDomainOpenGraphics

[Daniel P. Berrange]

From: "Daniel P. Berrange" 

The virDomainOpenGraphics method accepts a UNIX socket FD from
the client app. It must set the label on this FD otherwise QEMU
will be prevented from receiving it with recvmsg.

Signed-off-by: Daniel P. Berrange 
---
 src/qemu/qemu_driver.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5124f27..0a8e518 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14777,6 +14777,10 @@ qemuDomainOpenGraphics(virDomainPtr dom,
 goto cleanup;
 }
 
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd) < 0)
+goto cleanup;
+
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 qemuDomainObjEnterMonitor(driver, vm);
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Paolo Bonzini]

Il 22/08/2013 12:34, Laszlo Ersek ha scritto:
> 

Actually it's fine to clarify these things!  Hence the longish
digression below.

> I think before priority comes into the picture, the access size would
> matter first, no?
> 
> (I think I'm recalling this from the 0xCF9 reset control register, which
> falls into the [0xCF8..0xCFA] range.)

The base address is what matters.  A 2- or 4-byte access to x will
always go to the region that includes address x, even if there are other
regions between x and respectively x+1 or x+3.  So an access to 0xCF8
will go to the PCI address register, while an access to 0xCF9 will
always go to the reset control register.

This happens in address_space_translate_internal:

diff = int128_sub(section->mr->size, int128_make64(addr));

For a write to 0xCF8, addr would be 0 (it is relative to the base of the
MemoryRegion).  section->size would be 1 because the next section starts
at 0xCF9.  However, section->mr->size would be 4 as specified e.g. in
i440fx_pcihost_initfn:

memory_region_init_io(&s->conf_mem, obj, &pci_host_conf_le_ops, s,
  "pci-conf-idx", 4);

Using section->size would be wrong---it would attempt a 1-byte write to
0xCF8, another 1-byte write to 0xCF9, and a 2-byte write to 0xCFA.
section->mr->size instead does a single write to 0xCF8, the same as on
real hardware.

BTW, the behavior changed slightly in QEMU 1.6 for 8-byte accesses. All
such accesses were split to two 4-byte accesses before; now the maximum
size of a "direct" MMIO operation (the data bus size, effectively) is 64
bits, so a 64-bit write will always address a single MemoryRegion.

For example, say you had the PCI address and data registers occupying
two separate 4-byte MemoryRegions in 8 consecutive bytes of memory.  In
1.5 you could write both of them with a single 64-bit write.  In 1.6,
this would only write four bytes to the first MemoryRegion.  This
matches hardware more closely, and is really unlikely to be a problem: a
target with 32-bit data bus probably would not have 64-bit CPU registers
to begin with.  If it did, it would resemble the architecture of the
80386SX or 8088 processors.

> Unless ioport 0 is accessed with width 1 for dma-chan purposes, I think
> such an access would be unique to pvpanic, and always dispatched to pvpanic.

It is:

static const MemoryRegionOps channel_io_ops = {
.read = read_chan,
.write = write_chan,
.endianness = DEVICE_NATIVE_ENDIAN,
.impl = {
.min_access_size = 1,
.max_access_size = 1,
},
};

>> Channel 0 is (was) used for DRAM refresh, so
>> it should not have any visible effect.  However, it may not be entirely
>> disabling pvpanic, just making it mostly invisible.
> 
> That's good enough for the guest to reach kexec :)

Yes, I cannot deny that. :)

Paolo

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

Re: [libvirt] [PATCH] docs: Reformat attribute description in formatdomain

[John Ferlan]

On 08/20/2013 03:41 PM, John Ferlan wrote:
> Reformat the description to more cleanly delineate the attributes
> for a  element.
> ---
> 
> Similar to the recently changed  attribute.
> 
>  docs/formatdomain.html.in | 121 
> +++---
>  1 file changed, 71 insertions(+), 50 deletions(-)
> 

pushed - thanks,

John

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

Re: [libvirt] [PATCH] tests: Add URI precedence checking

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 12:58:46PM +0200, Martin Kletzander wrote:
> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
> to deal with this issue again, I present you The "uriprecedencetest".
> 
> Signed-off-by: Martin Kletzander 
> ---
>  tests/Makefile.am   |  2 ++
>  tests/uriprecedencetest | 77 
> +
>  2 files changed, 79 insertions(+)
>  create mode 100755 tests/uriprecedencetest
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 09144d6..419db8b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -98,6 +98,7 @@ EXTRA_DIST =\
>   storagevolxml2xmlout \
>   sysinfodata \
>   test-lib.sh \
> + uriprecedencetest \
>   vmx2xmldata \
>   xencapsdata \
>   xmconfigdata \
> @@ -235,6 +236,7 @@ test_scripts +=   \
>   read-bufsiz \
>   read-non-seekable   \
>   start   \
> + uriprecedencetest   \

Can you add a 'virsh-' prefix on this so we keep some grouping
of virsh tests

>   vcpupin \
>   virsh-all   \
>   virsh-optparse  \
> diff --git a/tests/uriprecedencetest b/tests/uriprecedencetest
> new file mode 100755
> index 000..9515e63
> --- /dev/null
> +++ b/tests/uriprecedencetest
> @@ -0,0 +1,77 @@
> +#!/bin/sh
> +
> +: ${srcdir=.}
> +. $srcdir/test-lib.sh
> +
> +# This test checks if virsh obeys the proper precedence of different
> +# URI settings
> +test_intro "uriprecedencetest"
> +
> +virsh_bin="$abs_top_builddir/tools/virsh"
> +counter=1
> +ret=0
> +
> +tmpdir="$(mktemp -d)"
> +cleanup_() { rm -rf "$tmpdir"; }

IIIUC,  test-lib.sh is creating a temporary directory in which
the test is run. So I think we can just use CWD for our state
files, rather than creating a new tmpdir.

> +unset LIBVIRT_DEFAULT_URI
> +unset VIRSH_DEFAULT_CONNECT_URI
> +bad_uri="test:///default?bad_uri"
> +good_uri="test:///default?good_uri"

Ahh, clever idea to invent custom uris :-)

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

[libvirt] [PATCH] tests: Add URI precedence checking

[Martin Kletzander]

Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
to deal with this issue again, I present you The "uriprecedencetest".

Signed-off-by: Martin Kletzander 
---
 tests/Makefile.am   |  2 ++
 tests/uriprecedencetest | 77 +
 2 files changed, 79 insertions(+)
 create mode 100755 tests/uriprecedencetest

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 09144d6..419db8b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -98,6 +98,7 @@ EXTRA_DIST =  \
storagevolxml2xmlout \
sysinfodata \
test-lib.sh \
+   uriprecedencetest \
vmx2xmldata \
xencapsdata \
xmconfigdata \
@@ -235,6 +236,7 @@ test_scripts += \
read-bufsiz \
read-non-seekable   \
start   \
+   uriprecedencetest   \
vcpupin \
virsh-all   \
virsh-optparse  \
diff --git a/tests/uriprecedencetest b/tests/uriprecedencetest
new file mode 100755
index 000..9515e63
--- /dev/null
+++ b/tests/uriprecedencetest
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+: ${srcdir=.}
+. $srcdir/test-lib.sh
+
+# This test checks if virsh obeys the proper precedence of different
+# URI settings
+test_intro "uriprecedencetest"
+
+virsh_bin="$abs_top_builddir/tools/virsh"
+counter=1
+ret=0
+
+tmpdir="$(mktemp -d)"
+cleanup_() { rm -rf "$tmpdir"; }
+
+# Create all mock files/directories to avoid permission problems
+tmphome="$tmpdir/tmp_home"
+export XDG_CONFIG_HOME="$tmpdir/.config"
+export XDG_CACHE_HOME="$tmpdir/.cache"
+export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
+
+mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
+mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
+mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
+
+# Main function checking for the proper uri being returned
+test_uri()
+{
+  result=0
+  if [ "$($virsh_bin uri)" != "$good_uri" ]; then
+  result=1
+  ret=1
+  fi
+  test_result "$counter" "$1" "$result"
+  counter="$((counter+1))"
+}
+
+# Precedence is the following (lowest priority first):
+#
+# 1) if run as root, 'uri_default' from /etc/libvirtd/libvirt.conf,
+#otherwise qemu:///session.  There is no way to mock this file for
+#virsh/libvirt.so and the user may have set anything in there that
+#would spoil the test, so we don't test this
+#
+# 2) 'uri_default' from $XDG_CONFIG_HOME/libvirt/libvirt.conf
+#
+# 3) LIBVIRT_DEFAULT_URI
+#
+# 4) VIRSH_DEFAULT_CONNECT_URI
+#
+# 5) parameter -c (--connect)
+
+unset LIBVIRT_DEFAULT_URI
+unset VIRSH_DEFAULT_CONNECT_URI
+bad_uri="test:///default?bad_uri"
+good_uri="test:///default?good_uri"
+
+printf "uri_default=\"%s\"\n" "$good_uri" 
>"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
+test_uri "User config file"
+
+printf "uri_default=\"%s\"\n" "$bad_uri" 
>"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
+export LIBVIRT_DEFAULT_URI="$good_uri"
+test_uri "LIBVIRT_DEFAULT_URI"
+
+export LIBVIRT_DEFAULT_URI="$bad_uri"
+export VIRSH_DEFAULT_CONNECT_URI="$good_uri"
+test_uri "VIRSH_DEFAULT_CONNECT_URI"
+
+export VIRSH_DEFAULT_CONNECT_URI="$bad_uri"
+virsh_bin="$virsh_bin --connect $good_uri"
+test_uri "Parameter"
+
+# test_uri() increases $counter even for the last test, so we must
+# decrement it
+test_final "$((counter-1))" "$ret"
+(exit "$ret"); exit "$ret"
-- 
1.8.3.2

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

Re: [libvirt] [PATCH] qemuBuildNicDevStr: Add mq=on for multiqueue networking

[Laine Stump]

On 08/22/2013 04:36 AM, Michal Privoznik wrote:
> If user requested multiqueue networking, beside multiple /dev/tap and
> /dev/vhost-net openings, we forgot to pass mq=on onto the -device
> virtio-net-pci command line. This is advised at:
>
>   http://www.linux-kvm.org/page/Multiqueue#Enable_MQ_feature
> ---
>
> Notes:
> Maybe worth backporting to maintainer branches too. The MQ was introduced 
> in
> the 1.0.6 release.
>
>  src/qemu/qemu_command.c | 7 ++-
>  src/qemu/qemu_command.h | 1 +
>  src/qemu/qemu_hotplug.c | 4 +++-
>  3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f151173..d73872a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4730,6 +4730,7 @@ qemuBuildNicDevStr(virDomainDefPtr def,
> virDomainNetDefPtr net,
> int vlan,
> int bootindex,
> +   bool multiqueue,
> virQEMUCapsPtr qemuCaps)
>  {
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -4782,6 +4783,8 @@ qemuBuildNicDevStr(virDomainDefPtr def,
>
> virDomainVirtioEventIdxTypeToString(net->driver.virtio.event_idx));
>  }
>  }
> +if (usingVirtio && multiqueue)
> +virBufferAddLit(&buf, ",mq=on");
>  if (vlan == -1)
>  virBufferAsprintf(&buf, ",netdev=host%s", net->info.alias);
>  else
> @@ -7275,7 +7278,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>  virCommandAddArgList(cmd, "-netdev", host, NULL);
>  }
>  if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> -if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex, qemuCaps)))
> +bool multiqueue = tapfdSize > 1 || vhostfdSize > 1;
> +if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex,

usually we put a blank line between variable declarations and code.

> +   multiqueue, qemuCaps)))
>  goto cleanup;
>  virCommandAddArgList(cmd, "-device", nic, NULL);
>  } else {
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 5c5c025..a9854a3 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -102,6 +102,7 @@ char * qemuBuildNicDevStr(virDomainDefPtr def,
>virDomainNetDefPtr net,
>int vlan,
>int bootindex,
> +  bool multiqueue,
>virQEMUCapsPtr qemuCaps);
>  
>  char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 478f331..b49073b 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -859,7 +859,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>  }
>  
>  if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> -if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0, 
> priv->qemuCaps)))
> +bool multiqueue = tapfdSize > 1 || vhostfdSize > 1;
> +if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0,

Same comment here.

ACK with those two blank lines added.

> +  multiqueue, priv->qemuCaps)))
>  goto try_remove;
>  } else {
>  if (!(nicstr = qemuBuildNicStr(net, NULL, vlan)))

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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Laszlo Ersek]

On 08/22/13 12:34, Laszlo Ersek wrote:

> (I think I'm recalling this from the 0xCF9 reset control register, which
> falls into the [0xCF8..0xCFA] range.)

[0xCF8..0xCFB], sigh

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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Laszlo Ersek]

On 08/22/13 11:19, Paolo Bonzini wrote:
> Il 22/08/2013 10:38, Laszlo Ersek ha scritto:
 To support 1.5, libvirt should simply be ready to react to unanticipated
 GUEST_PANICKED events.  reboot-on-panic will simply be broken for 1.5
 and Linux 3.10+ guests. :(
>> I'm probably misunderstanding the discussion, but it might be possible
>> to disable pvpanic even in 1.5 from the host side, with the following hack:
>>
>>   -global pvpanic.ioport=0
>>
>> In qemu, this will either configure a working pvpanic device on ioport
>> 0, or the pvpanic device will be genuinely broken. At least it doesn't
>> (obviously) break other stuff (in v1.5.2):
>>
>> (qemu) info mtree
>> I/O
>> - (prio 0, RW): io
>>   - (prio 0, RW): pvpanic
>>   -0007 (prio 0, RW): dma-chan
> 
> No, you're not misunderstanding the discussion.
> 
> Depending on the priorities of the pvpanic and legacy-DMA regions, it
> would break DMA channel 0. 



I think before priority comes into the picture, the access size would
matter first, no?

(I think I'm recalling this from the 0xCF9 reset control register, which
falls into the [0xCF8..0xCFA] range.)

Unless ioport 0 is accessed with width 1 for dma-chan purposes, I think
such an access would be unique to pvpanic, and always dispatched to pvpanic.

> Channel 0 is (was) used for DRAM refresh, so
> it should not have any visible effect.  However, it may not be entirely
> disabling pvpanic, just making it mostly invisible.

That's good enough for the guest to reach kexec :)



Laszlo

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

Re: [libvirt] [PATCH] LXC: mount a fresh /run directory for container

[Gao feng]

On 08/22/2013 06:18 PM, Daniel P. Berrange wrote:
> On Thu, Aug 22, 2013 at 06:07:50PM +0800, Gao feng wrote:
>> On 08/22/2013 05:41 PM, Daniel P. Berrange wrote:
>>> On Thu, Aug 22, 2013 at 08:57:49AM +0800, Gao feng wrote:
 On 08/21/2013 05:31 PM, Daniel P. Berrange wrote:
> On Wed, Aug 21, 2013 at 04:22:29PM +0800, Gao feng wrote:
>> The unix socket file /run/systemd/private is used to
>> send reboot/shutdown messages. and since this type of
>> unix sockets are not per net namespace , they are
>> global resources. systemctl in container can use
>> this unix socket to send shutdown message to the
>> systemd-shutdownd running on host. finally the
>> host will be poweroff.
>>
>> this problem occurs when container shares the same
>> root directory with host.
>>
>> this patch umount host's /run directory and mount
>> the /run directory of container as tmpfs.
>>
>> Signed-off-by: Gao feng 
>> ---
>>  src/lxc/lxc_container.c | 5 +
>>  1 file changed, 5 insertions(+)
>
> I don't think we should be doing this by default. IMHO this is something
> the mgmt app / admin should take care of it they want to have separate
> /run.
>
> You may be preventing access to the systemd socket by doing this, but
> equally you can be breaking any number of other valid use cases by
> hiding the host's /run

 We can't assume user know the root reason why shutdown in container will
 shut down the host. they don't know it's because of container shares the
 /run/ directory with host. This will confuse them and bring bad image to
 them. We have lxcContainerHasReboot in libvirt, and it did tell user that
 "Containerized reboot support is available", but the fact is reboot in
 container will reboot host.

 and the /run directory is mounted as tmpfs on host. it means the files
 under /run are temporary, I don't think it's meaningful to share these
 files with container.

 If someone really want to share host's /run directory with container, he
 should add this filesystem configuration to the domain xml.
>>>
>>> Quite simply, no.
>>>
>>> If the user asks for '/', then that's what they'll get. If they want
>>> to hide /run they can do so.
>>>
>>
>> Users don't know why sharing root directory with host will cause host
>> can be rebooted from container. pid namespace is enabled by libvirt lxc and
>> actually libvirt did say that "Containerized reboot support is available".
>> it's hard for user to find out that container shouldn't share /run directory
>> with host. it's easy for them to find out some files are leaked under /run
>> directory for container, and then add this filesystem configuration to the
>> domain xml file.
>>
>>
>> And I still think it really make no sense for container to share /run
>> directory with host.
>>
>>> What you're describing is a usability policy issue, solution to which
>>> belongs in the tools.
>>>
>>> If you are editting XML directly to configure guests, it is expected
>>> that you know what you are doing.
>>>
> Ultimately user namespace should prevent access to the systemd
> sockets for people wanting a secure setup without replacing /run
>

 Some people may think user namespace is too strict, they may dislike
 to enable user namespace, just like they may want share net namespace
 with host. They have rights to start a container which shares same
 user namespace with host.
>>>
>>> They have the ability to specify a new mount of /run if they so desire.
>>>
>>
>> Yep, but they don't know how to fix this reboot-problem until they read
>> this thread or some documents somewhere.
>>
>> I think though users know sharing root directory with host will bring bad 
>> influence.
>> I guess they must don't know this will make their host can be powered off by 
>> the
>> user in container.
> 
> The majority of users will not be creating XML configs for LXC
> directly. They will use tools like virt-sandbox-service, or
> virt-install, which can take care todo the right setup for their
> use cases.
> 
> 

Hmm, indeed two fujitsu guys asked me why reboot in container will cause host 
to be
rebooted...

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

Re: [libvirt] [PATCH] LXC: mount a fresh /run directory for container

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 06:07:50PM +0800, Gao feng wrote:
> On 08/22/2013 05:41 PM, Daniel P. Berrange wrote:
> > On Thu, Aug 22, 2013 at 08:57:49AM +0800, Gao feng wrote:
> >> On 08/21/2013 05:31 PM, Daniel P. Berrange wrote:
> >>> On Wed, Aug 21, 2013 at 04:22:29PM +0800, Gao feng wrote:
>  The unix socket file /run/systemd/private is used to
>  send reboot/shutdown messages. and since this type of
>  unix sockets are not per net namespace , they are
>  global resources. systemctl in container can use
>  this unix socket to send shutdown message to the
>  systemd-shutdownd running on host. finally the
>  host will be poweroff.
> 
>  this problem occurs when container shares the same
>  root directory with host.
> 
>  this patch umount host's /run directory and mount
>  the /run directory of container as tmpfs.
> 
>  Signed-off-by: Gao feng 
>  ---
>   src/lxc/lxc_container.c | 5 +
>   1 file changed, 5 insertions(+)
> >>>
> >>> I don't think we should be doing this by default. IMHO this is something
> >>> the mgmt app / admin should take care of it they want to have separate
> >>> /run.
> >>>
> >>> You may be preventing access to the systemd socket by doing this, but
> >>> equally you can be breaking any number of other valid use cases by
> >>> hiding the host's /run
> >>
> >> We can't assume user know the root reason why shutdown in container will
> >> shut down the host. they don't know it's because of container shares the
> >> /run/ directory with host. This will confuse them and bring bad image to
> >> them. We have lxcContainerHasReboot in libvirt, and it did tell user that
> >> "Containerized reboot support is available", but the fact is reboot in
> >> container will reboot host.
> >>
> >> and the /run directory is mounted as tmpfs on host. it means the files
> >> under /run are temporary, I don't think it's meaningful to share these
> >> files with container.
> >>
> >> If someone really want to share host's /run directory with container, he
> >> should add this filesystem configuration to the domain xml.
> > 
> > Quite simply, no.
> > 
> > If the user asks for '/', then that's what they'll get. If they want
> > to hide /run they can do so.
> > 
> 
> Users don't know why sharing root directory with host will cause host
> can be rebooted from container. pid namespace is enabled by libvirt lxc and
> actually libvirt did say that "Containerized reboot support is available".
> it's hard for user to find out that container shouldn't share /run directory
> with host. it's easy for them to find out some files are leaked under /run
> directory for container, and then add this filesystem configuration to the
> domain xml file.
> 
> 
> And I still think it really make no sense for container to share /run
> directory with host.
> 
> > What you're describing is a usability policy issue, solution to which
> > belongs in the tools.
> > 
> > If you are editting XML directly to configure guests, it is expected
> > that you know what you are doing.
> > 
> >>> Ultimately user namespace should prevent access to the systemd
> >>> sockets for people wanting a secure setup without replacing /run
> >>>
> >>
> >> Some people may think user namespace is too strict, they may dislike
> >> to enable user namespace, just like they may want share net namespace
> >> with host. They have rights to start a container which shares same
> >> user namespace with host.
> > 
> > They have the ability to specify a new mount of /run if they so desire.
> > 
> 
> Yep, but they don't know how to fix this reboot-problem until they read
> this thread or some documents somewhere.
> 
> I think though users know sharing root directory with host will bring bad 
> influence.
> I guess they must don't know this will make their host can be powered off by 
> the
> user in container.

The majority of users will not be creating XML configs for LXC
directly. They will use tools like virt-sandbox-service, or
virt-install, which can take care todo the right setup for their
use cases.


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

Re: [libvirt] [PATCH] lib: implement new API to retrieve the list of CPU models

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 12:06:55PM +0200, Giuseppe Scrivano wrote:
> "Daniel P. Berrange"  writes:
> 
> >> 2) as you said, support arch=NULL to get the full list (and now that
> >> I've thought more about it, I should change the error to
> >> VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal
> >> error).
> >
> > I'd strictly forbid arch=NULL in the API and wire protocol
> > (ie use remote_nonnull_string arch).
> 
> How do we get the set of supported architectures?  I'd like that for
> completeness sake we take this into account now, even if there is no
> real use (yet?).

The capabilities XML shows you all architectures that libvirt knows
about.

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

Re: [libvirt] [PATCH] lib: implement new API to retrieve the list of CPU models

[Giuseppe Scrivano]

"Daniel P. Berrange"  writes:

>> 2) as you said, support arch=NULL to get the full list (and now that
>> I've thought more about it, I should change the error to
>> VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal
>> error).
>
> I'd strictly forbid arch=NULL in the API and wire protocol
> (ie use remote_nonnull_string arch).

How do we get the set of supported architectures?  I'd like that for
completeness sake we take this into account now, even if there is no
real use (yet?).

Giuseppe

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

Re: [libvirt] [PATCH] LXC: mount a fresh /run directory for container

[Gao feng]

On 08/22/2013 05:41 PM, Daniel P. Berrange wrote:
> On Thu, Aug 22, 2013 at 08:57:49AM +0800, Gao feng wrote:
>> On 08/21/2013 05:31 PM, Daniel P. Berrange wrote:
>>> On Wed, Aug 21, 2013 at 04:22:29PM +0800, Gao feng wrote:
 The unix socket file /run/systemd/private is used to
 send reboot/shutdown messages. and since this type of
 unix sockets are not per net namespace , they are
 global resources. systemctl in container can use
 this unix socket to send shutdown message to the
 systemd-shutdownd running on host. finally the
 host will be poweroff.

 this problem occurs when container shares the same
 root directory with host.

 this patch umount host's /run directory and mount
 the /run directory of container as tmpfs.

 Signed-off-by: Gao feng 
 ---
  src/lxc/lxc_container.c | 5 +
  1 file changed, 5 insertions(+)
>>>
>>> I don't think we should be doing this by default. IMHO this is something
>>> the mgmt app / admin should take care of it they want to have separate
>>> /run.
>>>
>>> You may be preventing access to the systemd socket by doing this, but
>>> equally you can be breaking any number of other valid use cases by
>>> hiding the host's /run
>>
>> We can't assume user know the root reason why shutdown in container will
>> shut down the host. they don't know it's because of container shares the
>> /run/ directory with host. This will confuse them and bring bad image to
>> them. We have lxcContainerHasReboot in libvirt, and it did tell user that
>> "Containerized reboot support is available", but the fact is reboot in
>> container will reboot host.
>>
>> and the /run directory is mounted as tmpfs on host. it means the files
>> under /run are temporary, I don't think it's meaningful to share these
>> files with container.
>>
>> If someone really want to share host's /run directory with container, he
>> should add this filesystem configuration to the domain xml.
> 
> Quite simply, no.
> 
> If the user asks for '/', then that's what they'll get. If they want
> to hide /run they can do so.
> 

Users don't know why sharing root directory with host will cause host
can be rebooted from container. pid namespace is enabled by libvirt lxc and
actually libvirt did say that "Containerized reboot support is available".
it's hard for user to find out that container shouldn't share /run directory
with host. it's easy for them to find out some files are leaked under /run
directory for container, and then add this filesystem configuration to the
domain xml file.


And I still think it really make no sense for container to share /run
directory with host.

> What you're describing is a usability policy issue, solution to which
> belongs in the tools.
> 
> If you are editting XML directly to configure guests, it is expected
> that you know what you are doing.
> 
>>> Ultimately user namespace should prevent access to the systemd
>>> sockets for people wanting a secure setup without replacing /run
>>>
>>
>> Some people may think user namespace is too strict, they may dislike
>> to enable user namespace, just like they may want share net namespace
>> with host. They have rights to start a container which shares same
>> user namespace with host.
> 
> They have the ability to specify a new mount of /run if they so desire.
> 

Yep, but they don't know how to fix this reboot-problem until they read
this thread or some documents somewhere.

I think though users know sharing root directory with host will bring bad 
influence.
I guess they must don't know this will make their host can be powered off by the
user in container.

Thanks

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

Re: [libvirt] [PATCH]LXC: force container to enable network namespace

[Chen HanXiao]

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, August 22, 2013 5:42 PM
> To: Gao feng
> Cc: Chen Hanxiao; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH]LXC: force container to enable network namespace
> 
> On Thu, Aug 22, 2013 at 09:23:50AM +0800, Gao feng wrote:
> > On 08/21/2013 05:53 PM, Daniel P. Berrange wrote:
> > > On Wed, Aug 21, 2013 at 05:49:05PM +0800, Chen Hanxiao wrote:
> > >> From: Chen Hanxiao 
> > >>
> > >> If we don't enable network namespace, we could shutdown host
> > >> inside container by command 'shutdown', which is unacceptable.
> > >> This patch will force users to enable network namespace
> > >> before they start container.
> > >>
> > >> Signed-off-by: Chen Hanxiao 
> > >> ---
> > >>  src/lxc/lxc_container.c |6 ++
> > >>  1 files changed, 6 insertions(+), 0 deletions(-)
> > >>
> > >> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> > >> index c7a22ab..5631ad7 100644
> > >> --- a/src/lxc/lxc_container.c
> > >> +++ b/src/lxc/lxc_container.c
> > >> @@ -1937,6 +1937,12 @@ int lxcContainerStart(virDomainDefPtr def,
> > >>  if (lxcNeedNetworkNamespace(def)) {
> > >>  VIR_DEBUG("Enable network namespaces");
> > >>  cflags |= CLONE_NEWNET;
> > >> +} else {
> > >> +errno = EINVAL;
> > >> +
> > >> +virReportSystemError(errno, "%s",
> > >> + _("Network namespace needed"));
> > >> +return -1;
> > >>  }
> > >>
> > >>  pid = clone(lxcContainerChild, stacktop, cflags, &args);
> > >
> > > NACK.
> > >
> > > We explicitly want to allow containers sharing the host's network
> > > namespace. If you wish to prevent the problem you describe, then
> > > set the  feature in the XML, or configure virtual NICs
> > >
> >
> > At least we should give user some warning message or add a notification
> > about the probable reboot problem if user doesn't configure private
> > net namespace for container.
> 
> That can be documented in the drvlxc.html.in page
> 

Thanks. New patches will come soon.

> 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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Paolo Bonzini]

Il 22/08/2013 11:37, Michael S. Tsirkin ha scritto:
>> No, you're not misunderstanding the discussion.
>>
>> Depending on the priorities of the pvpanic and legacy-DMA regions, it
>> would break DMA channel 0.  Channel 0 is (was) used for DRAM refresh, so
>> it should not have any visible effect.  However, it may not be entirely
>> disabling pvpanic, just making it mostly invisible.
>>
>> Paolo
> 
> Ugh.
> 
> And now that Paolo pointed out that nothing terrible
> happens even when migrating from host with pvpanic
> enabled to host with pvpanic disabled, I'm inclined
> to think we should just disable pvpanic in 1.5.X.
> 
> Thoughts?

You'd obviously have no objection from me.

Paolo

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

Re: [libvirt] [PATCH]LXC: force container to enable network namespace

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 09:23:50AM +0800, Gao feng wrote:
> On 08/21/2013 05:53 PM, Daniel P. Berrange wrote:
> > On Wed, Aug 21, 2013 at 05:49:05PM +0800, Chen Hanxiao wrote:
> >> From: Chen Hanxiao 
> >>
> >> If we don't enable network namespace, we could shutdown host
> >> inside container by command 'shutdown', which is unacceptable.
> >> This patch will force users to enable network namespace
> >> before they start container.
> >>
> >> Signed-off-by: Chen Hanxiao 
> >> ---
> >>  src/lxc/lxc_container.c |6 ++
> >>  1 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> >> index c7a22ab..5631ad7 100644
> >> --- a/src/lxc/lxc_container.c
> >> +++ b/src/lxc/lxc_container.c
> >> @@ -1937,6 +1937,12 @@ int lxcContainerStart(virDomainDefPtr def,
> >>  if (lxcNeedNetworkNamespace(def)) {
> >>  VIR_DEBUG("Enable network namespaces");
> >>  cflags |= CLONE_NEWNET;
> >> +} else {
> >> +errno = EINVAL;
> >> +
> >> +virReportSystemError(errno, "%s",
> >> + _("Network namespace needed"));
> >> +return -1;
> >>  }
> >>  
> >>  pid = clone(lxcContainerChild, stacktop, cflags, &args);
> > 
> > NACK.
> > 
> > We explicitly want to allow containers sharing the host's network
> > namespace. If you wish to prevent the problem you describe, then
> > set the  feature in the XML, or configure virtual NICs
> > 
> 
> At least we should give user some warning message or add a notification
> about the probable reboot problem if user doesn't configure private
> net namespace for container.

That can be documented in the drvlxc.html.in page

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

Re: [libvirt] Is it a problem that "a vm has been shutdown, but its pidfile has not been removed"

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 09:34:55AM +, Wangyufei (A) wrote:
> Hello,
> when do steps as follows:
> 
> 1.  start a vm
> 
> 2.  restart libvirtd service
> 
> 3.  shutdown the vm
> I found the pidfile associated with the vm has not been removed.
> 
> Analyzing related piece of code, I found when libvirtd service 
> restarted, the vm's _virDomainObj structure(named dom here) in
> libvirtd will be re-established according to the vm's state-file 
> or config-file, but its member of privateData.pidfile will not be
> reassigned value. By the way, qemudriver is used in my libvirt.
> 
> So when the vm is shutdown, its pidfile will not be removed,
> because the member of privateData.pidfile is NULL.
> 
> My questions are :
> 
> 1.  Do you think it is a problem when a vm has been shutdown,
> but its pidfile has not been removed ?

It is fairly harmless, but it would be good practice to fix
this so it is removed.


> 2.  Do you think it is expected that the member of
> privateData.pidfile's value missed after libvirtd restart?
> 
> If it's a bug then I'll submit the patch to fix it.

Yes, that's a bug. When restarting libvirtd, we want to maintain
all the state we had before. So loosing the pidfile string is
a mistake.

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

Re: [libvirt] [PATCHv2] virBitmapParse: Fix behavior in case of error and fix up callers

[Peter Krempa]

On 08/21/13 18:32, Eric Blake wrote:
> On 08/21/2013 07:12 AM, Peter Krempa wrote:
>> Re-arrange the code so that the returned bitmap is always initialized to
>> NULL even on early failures and return an error message as some callers
>> are already expecting it. Fix up the rest not to shadow the error.
>> ---
>>
>> Version 2:
>>  - squashed cleanup of callers into this patch
>>

...

> Ugg - I don't like easy-to-trigger user-visible error messages that
> state "internal error".  I think VIR_ERR_INVALID_ARG works better.
> 
> ACK with that tweak.
> 

Right, INVALID_ARG is better. Fixed and pushed.

Peter




signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] LXC: mount a fresh /run directory for container

[Daniel P. Berrange]

On Thu, Aug 22, 2013 at 08:57:49AM +0800, Gao feng wrote:
> On 08/21/2013 05:31 PM, Daniel P. Berrange wrote:
> > On Wed, Aug 21, 2013 at 04:22:29PM +0800, Gao feng wrote:
> >> The unix socket file /run/systemd/private is used to
> >> send reboot/shutdown messages. and since this type of
> >> unix sockets are not per net namespace , they are
> >> global resources. systemctl in container can use
> >> this unix socket to send shutdown message to the
> >> systemd-shutdownd running on host. finally the
> >> host will be poweroff.
> >>
> >> this problem occurs when container shares the same
> >> root directory with host.
> >>
> >> this patch umount host's /run directory and mount
> >> the /run directory of container as tmpfs.
> >>
> >> Signed-off-by: Gao feng 
> >> ---
> >>  src/lxc/lxc_container.c | 5 +
> >>  1 file changed, 5 insertions(+)
> > 
> > I don't think we should be doing this by default. IMHO this is something
> > the mgmt app / admin should take care of it they want to have separate
> > /run.
> > 
> > You may be preventing access to the systemd socket by doing this, but
> > equally you can be breaking any number of other valid use cases by
> > hiding the host's /run
> 
> We can't assume user know the root reason why shutdown in container will
> shut down the host. they don't know it's because of container shares the
> /run/ directory with host. This will confuse them and bring bad image to
> them. We have lxcContainerHasReboot in libvirt, and it did tell user that
> "Containerized reboot support is available", but the fact is reboot in
> container will reboot host.
> 
> and the /run directory is mounted as tmpfs on host. it means the files
> under /run are temporary, I don't think it's meaningful to share these
> files with container.
> 
> If someone really want to share host's /run directory with container, he
> should add this filesystem configuration to the domain xml.

Quite simply, no.

If the user asks for '/', then that's what they'll get. If they want
to hide /run they can do so.

What you're describing is a usability policy issue, solution to which
belongs in the tools.

If you are editting XML directly to configure guests, it is expected
that you know what you are doing.

> > Ultimately user namespace should prevent access to the systemd
> > sockets for people wanting a secure setup without replacing /run
> >
> 
> Some people may think user namespace is too strict, they may dislike
> to enable user namespace, just like they may want share net namespace
> with host. They have rights to start a container which shares same
> user namespace with host.

They have the ability to specify a new mount of /run if they so desire.

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

Re: [libvirt] [PATCH] lib: implement new API to retrieve the list of CPU models

[Daniel P. Berrange]

On Wed, Aug 21, 2013 at 10:46:36PM +0200, Giuseppe Scrivano wrote:
> Eric Blake  writes:
> 
> > I'm not sure whether returning XML or a straight-up list makes more
> > sense.  If you used char ***models, then the user would get an array of
> > directly-usable strings, "486", "pentium", ..., instead of a document
> > that has to be parsed.  On the other hand, your idea of returning XML
> > lets us return information for multiple arches simultaneously. But do we
> > need that flexibility, since arch is also an input parameter?  Is the
> > idea that you pass arch=NULL to get the full list, or arch="x86" to get
> > the x86 subset of the xml?  Why not just make arch mandatory and return
> > char ***; but then you have the question of which arches are supported.
> 
> I've thought a bit about XML vs Array and whether specifying the arch in
> the returned XML snippet or not, and these are the reasons behind my
> choice:
> 
> 1) leave room for VIR_CONNECT_GET_CPU_MODEL_NAMES_EXPAND_FEATURES, in
> the same way as it is done by virConnectBaselineCPU.  It might turn
> useful in future to get all the data in one shot, without requiring
> additional round-trips for each model trough virConnectBaselineCPU.

I don't think returning a list of all cpus with all features is
an operation that will be common, or performance critical.
So do not want to see us replicating virConnectBaselineCPU
functionality here.

> 2) as you said, support arch=NULL to get the full list (and now that
> I've thought more about it, I should change the error to
> VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal
> error).

I'd strictly forbid arch=NULL in the API and wire protocol
(ie use remote_nonnull_string arch).

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

[libvirt] Is it a problem that "a vm has been shutdown, but its pidfile has not been removed"

[Wangyufei (A)]

Hello,
when do steps as follows:

1.  start a vm

2.  restart libvirtd service

3.  shutdown the vm
I found the pidfile associated with the vm has not been removed.

Analyzing related piece of code, I found when libvirtd service restarted, 
the vm's _virDomainObj structure(named dom here) in libvirtd will be 
re-established according to the vm's state-file or config-file, but its member 
of privateData.pidfile will not be reassigned value. By the way, qemudriver is 
used in my libvirt.

So when the vm is shutdown, its pidfile will not be removed, because the member 
of privateData.pidfile is NULL.

My questions are :

1.  Do you think it is a problem when a vm has been shutdown, but its pidfile 
has not been removed ?

2.  Do you think it is expected that the member of privateData.pidfile's value 
missed after libvirtd restart?

If it's a bug then I'll submit the patch to fix it.

Also list the related piece of code about removing the vm's pidfile in 
libvirt for your information:
 if (priv->pidfile &&
unlink(priv->pidfile) < 0 &&
errno != ENOENT)
VIR_WARN("Failed to remove PID file for %s: %s",
 vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));

Best Regards,
-WangYufei

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

Re: [libvirt] [PATCH] lib: implement new API to retrieve the list of CPU models

[Daniel P. Berrange]

On Wed, Aug 21, 2013 at 01:19:24PM -0600, Eric Blake wrote:
> On 08/21/2013 01:02 PM, Giuseppe Scrivano wrote:
> > The new function virConnectGetCPUModelNames allows to retrieve the list
> > of CPU models known by the hypervisor for a specific architecture.
> > 
> > Signed-off-by: Giuseppe Scrivano 
> > ---
> > I have collected your comments on my RFC patch into this new version.  I've
> > replaced "virConnectGetCPUMapDesc" with "virConnectGetCPUModelNames".
> 
> Good that the above paragraph is below the ---...
> 
> > 
> > The new function signature is:
> > 
> > int
> > virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char 
> > **models,
> >unsigned int flags);
> > 
> > It returns (in MODELS) the list of CPU models formatted as an XML document,
> > like:
> > 
> > 
> >   
> > 
> > 
> > 
> > 
> > 
> > 
> > ...
> >   
> > 
> 
> ...but this is useful 6 months down the road, and should be in the
> commit message proper, above the ---.
> 
> I'm not sure whether returning XML or a straight-up list makes more
> sense.  If you used char ***models, then the user would get an array of
> directly-usable strings, "486", "pentium", ..., instead of a document
> that has to be parsed.  On the other hand, your idea of returning XML
> lets us return information for multiple arches simultaneously. But do we
> need that flexibility, since arch is also an input parameter?  Is the
> idea that you pass arch=NULL to get the full list, or arch="x86" to get
> the x86 subset of the xml?  Why not just make arch mandatory and return
> char ***; but then you have the question of which arches are supported.
> 
> So, let's get agreement on the best design before worrying about
> implementation (I'm still 50/50 on whether xml vs. char*** makes more
> sense, without more discussion to sway me one way or the other).

My intention was that we return a direct list of strings representing
model names, *not* any XML.

XML only comes into play when they they query the full CPU feature
set from virConnectBaselineCPU.


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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Michael S. Tsirkin]

On Thu, Aug 22, 2013 at 11:19:44AM +0200, Paolo Bonzini wrote:
> Il 22/08/2013 10:38, Laszlo Ersek ha scritto:
> >> > To support 1.5, libvirt should simply be ready to react to unanticipated
> >> > GUEST_PANICKED events.  reboot-on-panic will simply be broken for 1.5
> >> > and Linux 3.10+ guests. :(
> > I'm probably misunderstanding the discussion, but it might be possible
> > to disable pvpanic even in 1.5 from the host side, with the following hack:
> > 
> >   -global pvpanic.ioport=0
> > 
> > In qemu, this will either configure a working pvpanic device on ioport
> > 0, or the pvpanic device will be genuinely broken. At least it doesn't
> > (obviously) break other stuff (in v1.5.2):
> > 
> > (qemu) info mtree
> > I/O
> > - (prio 0, RW): io
> >   - (prio 0, RW): pvpanic
> >   -0007 (prio 0, RW): dma-chan
> 
> No, you're not misunderstanding the discussion.
> 
> Depending on the priorities of the pvpanic and legacy-DMA regions, it
> would break DMA channel 0.  Channel 0 is (was) used for DRAM refresh, so
> it should not have any visible effect.  However, it may not be entirely
> disabling pvpanic, just making it mostly invisible.
> 
> Paolo

Ugh.

And now that Paolo pointed out that nothing terrible
happens even when migrating from host with pvpanic
enabled to host with pvpanic disabled, I'm inclined
to think we should just disable pvpanic in 1.5.X.

Thoughts?

-- 
MST

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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Paolo Bonzini]

Il 22/08/2013 10:38, Laszlo Ersek ha scritto:
>> > To support 1.5, libvirt should simply be ready to react to unanticipated
>> > GUEST_PANICKED events.  reboot-on-panic will simply be broken for 1.5
>> > and Linux 3.10+ guests. :(
> I'm probably misunderstanding the discussion, but it might be possible
> to disable pvpanic even in 1.5 from the host side, with the following hack:
> 
>   -global pvpanic.ioport=0
> 
> In qemu, this will either configure a working pvpanic device on ioport
> 0, or the pvpanic device will be genuinely broken. At least it doesn't
> (obviously) break other stuff (in v1.5.2):
> 
> (qemu) info mtree
> I/O
> - (prio 0, RW): io
>   - (prio 0, RW): pvpanic
>   -0007 (prio 0, RW): dma-chan

No, you're not misunderstanding the discussion.

Depending on the priorities of the pvpanic and legacy-DMA regions, it
would break DMA channel 0.  Channel 0 is (was) used for DRAM refresh, so
it should not have any visible effect.  However, it may not be entirely
disabling pvpanic, just making it mostly invisible.

Paolo

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

Re: [libvirt] [RFC PATCH v2 0/3] Start fixing the pvpanic mess

[Daniel P. Berrange]

On Wed, Aug 21, 2013 at 06:56:52PM +0200, Paolo Bonzini wrote:
> Il 21/08/2013 18:55, Daniel P. Berrange ha scritto:
> > On Wed, Aug 21, 2013 at 06:51:11PM +0200, Paolo Bonzini wrote:
> >> Il 21/08/2013 18:48, Daniel P. Berrange ha scritto:
> >>> No,  is the right thing to be using for this from
> >>> libvirt's pov & I don't think we should invent something new.
> >>> The  element has always been intended to represent
> >>> handling of guest panics, not qemu internal errors.
> >>
> >> Actually for Xen HVM guests, it mostly traps things such as failed
> >> vmentries.  The Xen PV-on-HVM drivers do not register a panic notifier
> >> that moves the guest to the "crashed" state.
> >>
> >>  cannot be salvaged, in my opinion, because all domain XMLs in
> >> the wild will have a setting that causes libvirt to add "-device
> >> isa-pvpanic".  Thus changing libvirt versions will change guest
> >> hardware, which is _very_ bad.
> >>
> >> In addition, Windows XP and 2003 will show the annoying device wizard
> >> upon a libvirt upgrade, and fixing this is what surfaced all the mess.
> > 
> > The existance of a  element should not be having any
> > effect on what hardware we create. That is merely a lifecycle
> > policy setting that should be completely independant of the
> > guest device model.
> > 
> > eg it is valid to have  present in the XML at all
> > times, even if there's no pvpanic device present. That simply
> > means the actions will never be triggered.
> 
> So are you suggesting to add a  element to ?  That
> may be fine, but it doesn't seem very user-friendly.

Yes, if we're going to have pvpanic be user controllable, it must be
via an explicit device element. 

None of the  elements should have any impact on guest ABI
model. They're purely lifecycle policy settings.

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

Re: [libvirt] [PATCH] vl: allow "cont" from panicked state

[Laszlo Ersek]

On 08/21/13 17:32, Paolo Bonzini wrote:

> To support 1.5, libvirt should simply be ready to react to unanticipated
> GUEST_PANICKED events.  reboot-on-panic will simply be broken for 1.5
> and Linux 3.10+ guests. :(

I'm probably misunderstanding the discussion, but it might be possible
to disable pvpanic even in 1.5 from the host side, with the following hack:

  -global pvpanic.ioport=0

In qemu, this will either configure a working pvpanic device on ioport
0, or the pvpanic device will be genuinely broken. At least it doesn't
(obviously) break other stuff (in v1.5.2):

(qemu) info mtree
I/O
- (prio 0, RW): io
  - (prio 0, RW): pvpanic
  -0007 (prio 0, RW): dma-chan

(qemu) info qtree
bus: main-system-bus
  dev: i440FX-pcihost, id ""
bus: pci.0
  dev: PIIX3, id ""
bus: isa.0
  dev: pvpanic, id ""
ioport = 0

Either way, the "etc/pvpanic-port" fw_cfg file will contain 0, and
SeaBIOS will interpret it as "no pvpanic device". It will report the
same to the Linux guest too (_STA will return 0 for QEMU0001;
pvpanic_add() --> -ENODEV). Thus, no pvpanic notifier should be
registered, and reboot-on-panic should be reachable in the guest.

A horrible hack, certainly.

Laszlo

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