[libvirt] 答复: [PATCH 0/3] virSysinfo: Introduce SMBIOS type 3 support

[Zhuangyanying]

> -邮件原件-
> 发件人: John Ferlan [mailto:jfer...@redhat.com]
> 发送时间: 2018年2月14日 22:02
> 收件人: Zhuangyanying ;
> libvir-list@redhat.com; berra...@redhat.com
> 抄送: Zhangbo (Oscar) ; Gonglei (Arei)
> ; Jiangyifei 
> 主题: Re: [libvirt] [PATCH 0/3] virSysinfo: Introduce SMBIOS type 3 support
> 
> 
> 
> On 02/14/2018 04:22 AM, Zhuangyanying wrote:
> > From: Zhuang Yanying 
> >
> > Some applications inside VM need to access SMBIOS Chassis Asset Tag,
> > which should be emulated.
> >
> > access inside VM (for example)
> > Linux:   /sys/class/dmi/id/chassis_asset_tag.
> > Windows: (Get-WmiObject Win32_SystemEnclosure).SMBIOSAssetTag
> >   wirhin Windows PowerShell.
> >
> > It has already been realized in qemu:
> >
> > SMBIOS: Build aggregate smbios tables and entry point
> >
> https://git.qemu.org/?p=qemu.git;a=commit;h=c97294ec1b9e36887e119589d
> 456557d72ab37b5
> >
> > but not in libvirt. we realize it here.
> > As an example, you could use something like
> >
> > 
> >   Huawei
> >   To be filled by O.E.M.
> >   To be filled by O.E.M.
> >   To be filled by O.E.M.
> 
> Would prefer some more "realistic values" rather than "To be filled by
> O.E.M."...  They don't have to be exactly what is on your system, but
> closer to expectations would be nice.  Similar to what already exists.
> 
> You can just respond here and I can make the changes for you.
> 
Thank you very much for your help !
Whether the configuration below is appropriate:
 
   Dell Inc.
   2.12
   65X0XF2
   4101
   Type3Sku1


> NB: The xml files you put in patch2 should have been in patch1 - I can
> move those too.
> 
Oh, yes, I missed it.
Thanks again for your help !

Regards,
-Zhuang Yanying

> Other than that everything looks good to me.
> 
> John
> 
> >   Type3Sku1
> > 
> >
> > BTW: I'll be on vacation for china spring festival for the next week, I'll
> response as soon as I get back if there's any modification needed.
> >
> > Zhuang Yanying (3):
> >   conf: add support for setting Chassis SMBIOS data fields
> >   qemu: add support for generating SMBIOS Chassis strings command line
> >   news: add support for setting Chassis SMBIOS data fields
> >
> >  docs/formatdomain.html.in   |  23 +++
> >  docs/news.xml   |   5 ++
> >  docs/schemas/domaincommon.rng   |  22 ++
> >  src/conf/domain_conf.c  |  55 +++
> >  src/libvirt_private.syms|   1 +
> >  src/qemu/qemu_command.c |  51 ++
> >  src/util/virsysinfo.c   | 133
> +++-
> >  src/util/virsysinfo.h   |  13 
> >  tests/qemuxml2argvdata/smbios.args  |   2 +
> >  tests/qemuxml2argvdata/smbios.xml   |   7 ++
> >  tests/qemuxml2xmloutdata/smbios.xml |   7 ++
> >  11 files changed, 318 insertions(+), 1 deletion(-)
> >

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

Re: [libvirt] [PATCH 0/9] Resolve libvirtd hang on termination with connected long running client

[John Ferlan]

On 01/26/2018 03:47 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 19.01.2018 20:23, John Ferlan wrote:
>> RFC:
>> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>>
>> Adjustments since RFC...
>>
>> Patches 1&2: No change, were already R-B'd
>> Patch 3: Removed code as noted in code review, update commit message
>> Patch 4: From old series removed, see below for more details
>> Patch 9: no change
>> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy 
>> 
>> are removed as they seemed to not be necessary
>>
>> Replaced the former patch 4 with series of patches to (slowly) provide
>> support to disable new connections, handle removing waiting jobs, causing
>> the waiting workers to quit, and allow any running jobs to complete.
>>
>> As it turns out, waiting for running jobs to complete cannot be done
>> from the virNetServerClose callbacks because that means the event loop
>> processing done during virNetServerRun will not allow any currently
>> long running worker job thread a means to complete.
> 
> Hi, John.

Sorry - been busy in other areas lately - trying to get back to this...

> 
> So you suggest a different appoarch. Instead of introducing means to 
> help rpc threads to finish after event loop is finished (stateShutdown hook 
> in my series)
> you suggest to block futher new rpc processing and then waiting running
> rpc calls to finish keeping event loop running for that purpose.

Somewhere along the way in one of the reviews it was suggested to give
the workers perhaps a few extra cycles to complete anything they might
be doing. It was also suggested a mechanism to do that - which is what I
tried to accomplish here.

Based on that and that your mechanism was more of an "immediate
separation" by IIRC closing the monitor connection and letting that
close handler do whatever magic happens there.

This not to say your mechanism is the wrong way to go about it, but it
also hasn't garnered support nor have you actively attempted to champion
it. So I presented an alternative approach.

> 
> This could lead to libvirtd never finish its termination if one of
> qemu processes do not respond for example. In case of long running
> operation such as migration finishing can take much time. On the 
> over hand if we finish rpc threads abruptly as in case of stateShutdown hook
> we will deal with all possible error paths on daemon finishing. May
> be good approach is to abort long running operation, then give rpc threads
> some time to finish as you suggest and only after that finish them abruptly 
> to handle
> hanging qemu etc.

True, but it's also easy enough to add something to the last and updated
patch to add the QuitTimer and force an exit without going through the
rest of the "friendly" quit (IOW: more or less what Dan has suggested).

There's an incredible amount of cycles being spent for what amounts to
trying to be nice from a SIGTERM, SIGQUIT, or SIGINT - IOW: eventual death.


> 
> Also in this approach we keep event loop running for rpc threads only 
> but there can be other threads that depend on the loop. For example if 
> qemu is rebooted we spawn a thread that executes qemuProcessFakeReboot
> that sends commands to qemu (i.e. depends on event loop). We need to await
> such threads finishing too while keep event loop running. In approach of
> stateShutdown hook we finish all threads that uses qemu monitor by closing
> the monitor.
> 
> And hack with timeout in a loop... I think of a different aproach for waiting
> rpc threads to finish their work. First let's make drain function only flush
> job queue and take some callback which will be called when all workers will
> be free from work (let's keep worker threads as Dan suggested). In this 
> callback we
> can use same technique as in virNetDaemonSignalHandler. That is make some
> IO to set some flag in the event loop thread and cause virEventRunDefaultImpl
> to finish and then test this flag in virNetDaemonRun.
> 

Please feel free to spend as many cycles as you can making adjustments
to what I have. I'm working through some alterations and posting a v2 of
what I have and we'll see where it takes me/us.

John


> Nikolay
> 
>>
>> So when virNetDaemonQuit is called as a result of the libvirtd signal
>> handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
>> to quit immediately, alter to using a quitRequested flag and then use
>> that quitRequested flag to check for long running worker threads before
>> causing the event loop to quit resulting in libvirtd being able to run
>> through the virNetDaemonClose processing.
>>
>> John Ferlan (9):
>>   libvirtd: Alter refcnt processing for domain server objects
>>   libvirtd: Alter refcnt processing for server program objects
>>   netserver: Remove ServiceToggle during ServerDispose
>>   util: Introduce virThreadPoolDrain
>>   rpc: Introduce virNetServerQuitRequested
>>   rpc: Introduce virNetServerWorkerCount
>>   rpc: Alter 

[libvirt] [PATCH v2 1/2] conf, qemu: Check for NULL addrs in virDomainUSBAddressRelease

[John Ferlan]

Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0. This also removes the
need for ATTRIBUTE_NONNULL which only really helped if someone
passed a NULL as a parameter not if the passed parameter is NULL.

Signed-off-by: John Ferlan 
---
 src/conf/domain_addr.c |  2 +-
 src/conf/domain_addr.h |  2 +-
 src/qemu/qemu_domain_address.c |  7 ++-
 src/qemu/qemu_hotplug.c| 10 +++---
 4 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 642268239..a44f96470 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -2177,7 +2177,7 @@ virDomainUSBAddressRelease(virDomainUSBAddressSetPtr 
addrs,
 int targetPort;
 int ret = -1;
 
-if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB ||
+if (!addrs || info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB ||
 !virDomainUSBAddressPortIsValid(info->addr.usb.port))
 return 0;
 
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 173101465..7565322bd 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -330,5 +330,5 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
 int
 virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ATTRIBUTE_NONNULL(2);
 #endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index e28119e4b..613a27ef5 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2918,11 +2918,8 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
 if (virDeviceInfoPCIAddressPresent(info))
 virDomainPCIAddressReleaseAddr(priv->pciaddrs, >addr.pci);
 
-if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
-priv->usbaddrs &&
-virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
-VIR_WARN("Unable to release USB address on %s",
- NULLSTR(devstr));
+if (virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
+VIR_WARN("Unable to release USB address on %s", NULLSTR(devstr));
 }
 
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c9868de77..27bda3db1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2253,7 +2253,6 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 char *devstr = NULL;
-bool releaseaddr = false;
 bool added = false;
 bool teardowncgroup = false;
 bool teardownlabel = false;
@@ -2262,8 +2261,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
 
 if (priv->usbaddrs) {
 if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
-goto cleanup;
-releaseaddr = true;
+return -1;
 }
 
 if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, , 1, 0) < 
0)
@@ -2316,8 +2314,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
 VIR_WARN("Unable to remove host device from /dev");
 if (added)
 qemuHostdevReAttachUSBDevices(driver, vm->def->name, , 1);
-if (releaseaddr)
-virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
+virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
 }
 VIR_FREE(devstr);
 return ret;
@@ -3818,8 +3815,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
 dev.type = VIR_DOMAIN_DEVICE_DISK;
 dev.data.disk = disk;
 ignore_value(qemuRemoveSharedDevice(driver, , vm->def->name));
-if (priv->usbaddrs)
-virDomainUSBAddressRelease(priv->usbaddrs, >info);
+virDomainUSBAddressRelease(priv->usbaddrs, >info);
 
 virDomainDiskDefFree(disk);
 return 0;
-- 
2.13.6

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

[libvirt] [PATCH v2 2/2] conf, qemu: Check for NULL addrs in virDomainUSBAddressEnsure

[John Ferlan]

Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0. This also removes the
need for ATTRIBUTE_NONNULL which only really helped if someone
passed a NULL as a parameter not if the passed parameter is NULL.

Signed-off-by: John Ferlan 
---
 src/conf/domain_addr.c  |  3 +++
 src/conf/domain_addr.h  |  2 +-
 src/qemu/qemu_hotplug.c | 23 ---
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index a44f96470..78ff7a9cc 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -2154,6 +2154,9 @@ int
 virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
   virDomainDeviceInfoPtr info)
 {
+if (!addrs)
+return 0;
+
 if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
 (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
  !virDomainUSBAddressPortIsValid(info->addr.usb.port))) {
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 7565322bd..d3541bab0 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -325,7 +325,7 @@ virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
 int
 virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
   virDomainDeviceInfoPtr info)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ATTRIBUTE_NONNULL(2);
 
 int
 virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 27bda3db1..57fa035b8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -681,10 +681,8 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 
-if (priv->usbaddrs) {
-if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
-return -1;
-}
+if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
+return -1;
 
 if (qemuDomainAttachDiskGeneric(conn, driver, vm, disk) < 0) {
 virDomainUSBAddressRelease(priv->usbaddrs, >info);
@@ -1837,8 +1835,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm,
 return -1;
 return 1;
 
-} else if (priv->usbaddrs &&
-   chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
 if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
 return -1;
@@ -2259,10 +2256,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
 bool teardowndevice = false;
 int ret = -1;
 
-if (priv->usbaddrs) {
-if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
-return -1;
-}
+if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
+return -1;
 
 if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, , 1, 0) < 
0)
 goto cleanup;
@@ -2854,11 +2849,9 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver,
 if (qemuDomainEnsureVirtioAddress(, vm, , "input") < 0)
 return -1;
 } else if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) {
-if (priv->usbaddrs) {
-if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
-goto cleanup;
-releaseaddr = true;
-}
+if (virDomainUSBAddressEnsure(priv->usbaddrs, >info) < 0)
+goto cleanup;
+releaseaddr = true;
 }
 
 if (qemuAssignDeviceInputAlias(vm->def, input, -1) < 0)
-- 
2.13.6

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

[libvirt] [PATCH v2 0/2] Couple of hotplug cleanups

[John Ferlan]

Necromancing some old branches... 

v1: https://www.redhat.com/archives/libvir-list/2017-October/msg00930.html

Changes since v1 - beef up commit messages and drop patches 3 & 4 as
that's a bit more involved and Jan seemed to imply he eventually may
send patches dealing with the same thing - so I'll wait to see those.

John Ferlan (2):
  conf,qemu: Check for NULL addrs in virDomainUSBAddressRelease
  conf,qemu: Check for NULL addrs in virDomainUSBAddressEnsure

 src/conf/domain_addr.c |  5 -
 src/conf/domain_addr.h |  4 ++--
 src/qemu/qemu_domain_address.c |  7 ++-
 src/qemu/qemu_hotplug.c| 31 ++-
 4 files changed, 18 insertions(+), 29 deletions(-)

-- 
2.13.6

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

Re: [libvirt] [jenkins-ci PATCH] jobs: Set $PYTHONPATH for python-distutils jobs

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 05:53:57PM +0100, Andrea Bolognani wrote:
> Since we install Python modules under $VIRT_PREFIX, we need to set
> $PYTHONPATH or the interpreter won't be able to locate them. This is
> currently being done per-worker in the Jenkins Web interface.
> 
> However, now that we've introduced Python 3 builds, depending on the
> project and the OS, we might be using any of Python 2.7, 3.4, 3.5
> and 3.6, which means we can't have a single $PYTHONPATH anymore: in
> particular, while it's okay to have non-esisting directories in
> $PYTHONPATH, we have to make sure that a Python 3 interpreter will
> never try to use a Python 2 module and vice versa.
> 
> To solve the issue, we use a fairly large hammer: we set $PYTHONPATH
> at the job level, and include all reasonable minor versions for the
> Python major version (pyver) the job is using, even if they don't
> yet exist.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> Another approach would be to use something like
> 
>   T=$VIRT_PREFIX/lib/python{pyver}.4/site-packages
>   T=$T:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages
>   T=$T:$VIRT_PREFIX/lib/python{pyver}.5/site-packages
>   T=$T:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages
>   T=$T:$VIRT_PREFIX/lib/python{pyver}.6/site-packages
>   T=$T:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages
>   T=$T:$VIRT_PREFIX/lib/python{pyver}.7/site-packages
>   T=$T:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
>   export PYTHONPATH=$T
> 
> but that's just a different kind of ugly :/
> 
>  jobs/python-distutils.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
> index ff68c29..510769e 100644
> --- a/jobs/python-distutils.yaml
> +++ b/jobs/python-distutils.yaml
> @@ -42,6 +42,7 @@
>- shell: |
>{global_env}
>{local_env}
> +  export 
> PYTHONPATH=$VIRT_PREFIX/lib/python{pyver}.4/site-packages:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages:$VIRT_PREFIX/lib/python{pyver}.5/site-packages:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages:$VIRT_PREFIX/lib/python{pyver}.6/site-packages:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages:$VIRT_PREFIX/lib/python{pyver}.7/site-packages:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
>{command_pre_build}
>python{pyver} ./setup.py build
>python{pyver} ./setup.py install --prefix=$VIRT_PREFIX
> @@ -83,6 +84,7 @@
>- shell: |
>{global_env}
>{local_env}
> +  export 
> PYTHONPATH=$VIRT_PREFIX/lib/python{pyver}.4/site-packages:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages:$VIRT_PREFIX/lib/python{pyver}.5/site-packages:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages:$VIRT_PREFIX/lib/python{pyver}.6/site-packages:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages:$VIRT_PREFIX/lib/python{pyver}.7/site-packages:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
>python{pyver} ./setup.py test
>  publishers:
>- email:
> @@ -121,6 +123,7 @@
>- shell: |
>{global_env}
>{local_env}
> +  export 
> PYTHONPATH=$VIRT_PREFIX/lib/python{pyver}.4/site-packages:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages:$VIRT_PREFIX/lib/python{pyver}.5/site-packages:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages:$VIRT_PREFIX/lib/python{pyver}.6/site-packages:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages:$VIRT_PREFIX/lib/python{pyver}.7/site-packages:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
>sed -i -e 's/BuildRequires: libvirt.*devel.*//' *.spec.in
>python{pyver} ./setup.py rpm
>  publishers:

Reviewed-by: Daniel P. Berrangé 


Might as well kill PYTHONPATH from the nodes after activating this, to
avoid accidents elswhere.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [jenkins-ci PATCH] jobs: Set $PYTHONPATH for python-distutils jobs

[Andrea Bolognani]

Since we install Python modules under $VIRT_PREFIX, we need to set
$PYTHONPATH or the interpreter won't be able to locate them. This is
currently being done per-worker in the Jenkins Web interface.

However, now that we've introduced Python 3 builds, depending on the
project and the OS, we might be using any of Python 2.7, 3.4, 3.5
and 3.6, which means we can't have a single $PYTHONPATH anymore: in
particular, while it's okay to have non-esisting directories in
$PYTHONPATH, we have to make sure that a Python 3 interpreter will
never try to use a Python 2 module and vice versa.

To solve the issue, we use a fairly large hammer: we set $PYTHONPATH
at the job level, and include all reasonable minor versions for the
Python major version (pyver) the job is using, even if they don't
yet exist.

Signed-off-by: Andrea Bolognani 
---
Another approach would be to use something like

  T=$VIRT_PREFIX/lib/python{pyver}.4/site-packages
  T=$T:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages
  T=$T:$VIRT_PREFIX/lib/python{pyver}.5/site-packages
  T=$T:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages
  T=$T:$VIRT_PREFIX/lib/python{pyver}.6/site-packages
  T=$T:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages
  T=$T:$VIRT_PREFIX/lib/python{pyver}.7/site-packages
  T=$T:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
  export PYTHONPATH=$T

but that's just a different kind of ugly :/

 jobs/python-distutils.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
index ff68c29..510769e 100644
--- a/jobs/python-distutils.yaml
+++ b/jobs/python-distutils.yaml
@@ -42,6 +42,7 @@
   - shell: |
   {global_env}
   {local_env}
+  export 
PYTHONPATH=$VIRT_PREFIX/lib/python{pyver}.4/site-packages:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages:$VIRT_PREFIX/lib/python{pyver}.5/site-packages:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages:$VIRT_PREFIX/lib/python{pyver}.6/site-packages:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages:$VIRT_PREFIX/lib/python{pyver}.7/site-packages:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
   {command_pre_build}
   python{pyver} ./setup.py build
   python{pyver} ./setup.py install --prefix=$VIRT_PREFIX
@@ -83,6 +84,7 @@
   - shell: |
   {global_env}
   {local_env}
+  export 
PYTHONPATH=$VIRT_PREFIX/lib/python{pyver}.4/site-packages:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages:$VIRT_PREFIX/lib/python{pyver}.5/site-packages:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages:$VIRT_PREFIX/lib/python{pyver}.6/site-packages:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages:$VIRT_PREFIX/lib/python{pyver}.7/site-packages:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
   python{pyver} ./setup.py test
 publishers:
   - email:
@@ -121,6 +123,7 @@
   - shell: |
   {global_env}
   {local_env}
+  export 
PYTHONPATH=$VIRT_PREFIX/lib/python{pyver}.4/site-packages:$VIRT_PREFIX/lib64/python{pyver}.4/site-packages:$VIRT_PREFIX/lib/python{pyver}.5/site-packages:$VIRT_PREFIX/lib64/python{pyver}.5/site-packages:$VIRT_PREFIX/lib/python{pyver}.6/site-packages:$VIRT_PREFIX/lib64/python{pyver}.6/site-packages:$VIRT_PREFIX/lib/python{pyver}.7/site-packages:$VIRT_PREFIX/lib64/python{pyver}.7/site-packages
   sed -i -e 's/BuildRequires: libvirt.*devel.*//' *.spec.in
   python{pyver} ./setup.py rpm
 publishers:
-- 
2.14.3

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

Re: [libvirt] Should we switch to a different JSON library?

[Pino Toscano]

On Monday, 12 February 2018 19:47:00 CET Ján Tomko wrote:
> On Mon, Feb 12, 2018 at 02:38:02PM +0100, Pino Toscano wrote:
> >On Tuesday, 7 November 2017 14:05:25 CET Martin Kletzander wrote:
> >>  - Jansson [3] - I really like this one.  The API seems very intuitive,
> >>  it has nice documentation [4] in readthedocs (and I'm
> >>  not talking about the visual style, but how easy is to
> >>  find information), it can be used for formatting JSON
> >>  in a similar way we are doing it.  It has json_auto_t
> >>  (optional) type that uses the attribute cleanup for
> >>  automatic scope dereference (just in case we want to
> >>  use it), it has iterators... did I tell you I like this
> >>  one a lot?
> >>
> >> What do you (others) think of switching the JSON library?  Do you know
> >> about any other projects that could be used considering license,
> >> platform support, etc.?  Also feel free to fix any mistakes I might have
> >> posted.  I double-checked it, but you know, "trust, but verify".
> >
> >FYI: libguestfs just switched to Jansson [1], so any version starting
> >from 1.39.1 will use it instead of Yajl.  In case of libguestfs, yajl
> >was used directly, without wrappers of any sort, and the switch was
> >straightforward.
> >
> >[1] 
> >https://github.com/libguestfs/libguestfs/commit/bd1c5c9f4dcf38458099db8a0bf4659a07ef055d
> >
> 
> I do have a working virjson.c implementation in my local git waiting to be
> polished and sent. The issues with libvirt were:
> * virjson.c storing numbers as a string
> * backwards compatibility (AFAIK we want to support building on
>   RHEL/CentOS 6, which did not have recent enough jansson - for the
>   _foreach macros, at least 2.5 is needed)
>   If we really need to maintain two implementations side-by-side,
>   one of them should have an expiration date.

RHEL/CentOS 6 do not seem to have jansson -- it's in EPEL, and its
version there is 2.9.
RHEL/CentOS 7 have jansson 2.10.

> I don't see any version check in that libguestfs commit, what are the
> compatibility requirements there?

I though the APIs we used were old enough, but apparently not for an
old distro I still had around (Mageia 5, EOL).  I just bumped the
minimum requirement in libguestfs to 2.7, which is old enough, and
IMHO good enough as baseline; see
https://repology.org/metapackage/jansson/versions

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] virsh: detach-disk: Add --print-xml switch

[Peter Krempa]

Similarly to other commands add an argument which allows to check the
XML which would be used to execute the operation instead.

Signed-off-by: Peter Krempa 
---
 tools/virsh-domain.c | 10 ++
 tools/virsh.pod  |  4 
 2 files changed, 14 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5a0e0c1b21..c10bf184f9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12432,6 +12432,10 @@ static const vshCmdOptDef opts_detach_disk[] = {
 VIRSH_COMMON_OPT_DOMAIN_CONFIG,
 VIRSH_COMMON_OPT_DOMAIN_LIVE,
 VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+{.name = "print-xml",
+ .type = VSH_OT_BOOL,
+ .help = N_("print XML document rather than attach the interface")
+},
 {.name = NULL}
 };

@@ -12487,6 +12491,12 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }

+if (vshCommandOptBool(cmd, "print-xml")) {
+vshPrint(ctl, "%s", disk_xml);
+functionReturn = true;
+goto cleanup;
+}
+
 if (flags != 0 || current)
 ret = virDomainDetachDeviceFlags(dom, disk_xml, flags);
 else
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 69cc42338d..8f0e8d74b0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3113,6 +3113,7 @@ I<--persistent>.

 =item B I I
 [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
+[I<--print-xml>]

 Detach a disk device from a domain. The I is the device as seen
 from the domain.
@@ -3130,6 +3131,9 @@ an offline domain, and like I<--live> I<--config> for a 
running domain.
 Note that older versions of virsh used I<--config> as an alias for
 I<--persistent>.

+If B<--print-xml> is specified, then the XML which would be used to detach the
+disk is printed instead.
+
 =item B I I [I<--mac mac>]
 [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]

-- 
2.15.0

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

[libvirt] [PATCH 2/3] util: storage: Remove detected authentication data for backing chains

[Peter Krempa]

We can't really detect all the authentication data in a sane manner for
disk backing chains. Since the old RBD parser parses it in some cases as
the argv->XML convertor requires it, we can't just drop it.

Instead clear any detected authentication data in the code paths related
to disk backing chain lookup and fix the tests to cope with the change.

https://bugzilla.redhat.com/show_bug.cgi?id=1544659

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c |  8 
 tests/virstoragetest.c| 15 +++
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 7f878039ba..440d2b3040 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3399,6 +3399,14 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
 goto error;

 virStorageSourceNetworkAssignDefaultPorts(ret);
+
+/* Some of the legacy parsers parse authentication data since they are
+ * also used in other places. For backing store detection the
+ * authentication data would be invalid anyways, so we clear it */
+if (ret->auth) {
+virStorageAuthDefFree(ret->auth);
+ret->auth = NULL;
+}
 }

 return ret;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 6eed7134ed..a39c00bab5 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -308,8 +308,7 @@ static const char testStorageChainFormat[] =
 "type:%d\n"
 "format:%d\n"
 "protocol:%s\n"
-"hostname:%s\n"
-"secret:%s\n";
+"hostname:%s\n";

 static int
 testStorageChain(const void *args)
@@ -374,8 +373,7 @@ testStorageChain(const void *args)
 data->files[i]->type,
 data->files[i]->format,
 
virStorageNetProtocolTypeToString(data->files[i]->protocol),
-NULLSTR(data->files[i]->hostname),
-NULLSTR(data->files[i]->secret)) < 0 ||
+NULLSTR(data->files[i]->hostname)) < 0 ||
 virAsprintf(,
 testStorageChainFormat, i,
 NULLSTR(elt->path),
@@ -386,8 +384,7 @@ testStorageChain(const void *args)
 elt->type,
 elt->format,
 virStorageNetProtocolTypeToString(elt->protocol),
-NULLSTR(elt->nhosts ? elt->hosts[0].name : NULL),
-NULLSTR(elt->auth ? elt->auth->username : NULL)) < 0) {
+NULLSTR(elt->nhosts ? elt->hosts[0].name : NULL)) < 0) 
{
 VIR_FREE(expect);
 VIR_FREE(actual);
 goto cleanup;
@@ -1361,9 +1358,6 @@ mymain(void)
 TEST_BACKING_PARSE("rbd:testshare:id=asdf:mon_host=example.com",
"\n"
"  \n"
-   "  \n"
-   "\n"
-   "  \n"
"\n");
 TEST_BACKING_PARSE("nbd:example.org:6000:exportname=blah",
"\n"
@@ -1529,9 +1523,6 @@ mymain(void)
 "}",
"\n"
"  \n"
-   "  \n"
-   "\n"
-   "  \n"
"\n");
 TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"rbd\","
"\"image\":\"test\","
-- 
2.15.0

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

[libvirt] [PATCH 3/3] virsh: Remove sub-element in virshFindDisk

[Peter Krempa]

Previously we've removed the data only in virshUpdateDiskXML when
changing the disk source for the CDROM since the backing store would be
invalid. Move the code into a separate function and callit from
virshFindDisk which is also used when detaching disk.

The detaching code does not necessarily need to get the full backing
chain since it will need to act on the one managed by libvirt anyways
and this also takes care of problems when parts of the backing store
were invalid due to buggy RBD detection code.

Signed-off-by: Peter Krempa 
---
 tools/virsh-domain.c | 34 +++---
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c10bf184f9..d158327bd7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12151,6 +12151,26 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
 return ret;
 }

+
+static void
+virshDiskDropBackingStore(xmlNodePtr disk_node)
+{
+xmlNodePtr tmp;
+
+for (tmp = disk_node->children; tmp; tmp = tmp->next) {
+if (tmp->type != XML_ELEMENT_NODE)
+continue;
+
+if (virXMLNodeNameEqual(tmp, "backingStore")) {
+xmlUnlinkNode(tmp);
+xmlFreeNode(tmp);
+
+return;
+}
+}
+}
+
+
 typedef enum {
 VIRSH_FIND_DISK_NORMAL,
 VIRSH_FIND_DISK_CHANGEABLE,
@@ -12228,6 +12248,8 @@ virshFindDisk(const char *doc,

 if (STREQ_NULLABLE(tmp, path)) {
 ret = xmlCopyNode(obj->nodesetval->nodeTab[i], 1);
+/* drop backing store since they are not needed here */
+virshDiskDropBackingStore(ret);
 VIR_FREE(tmp);
 goto cleanup;
 }
@@ -12266,7 +12288,6 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
 {
 xmlNodePtr tmp = NULL;
 xmlNodePtr source = NULL;
-xmlNodePtr backingStore = NULL;
 xmlNodePtr target_node = NULL;
 xmlNodePtr text_node = NULL;
 char *device_type = NULL;
@@ -12307,22 +12328,13 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
 if (virXMLNodeNameEqual(tmp, "target"))
 target_node = tmp;

-if (virXMLNodeNameEqual(tmp, "backingStore"))
-backingStore = tmp;
-
 /*
  * We've found all we needed.
  */
-if (source && target_node && backingStore)
+if (source && target_node)
 break;
 }

-/* drop the  subtree since it would become invalid */
-if (backingStore) {
-xmlUnlinkNode(backingStore);
-xmlFreeNode(backingStore);
-}
-
 if (type == VIRSH_UPDATE_DISK_XML_EJECT) {
 if (!source) {
 vshError(NULL, _("The disk device '%s' doesn't have media"), 
target);
-- 
2.15.0

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

[libvirt] [PATCH 0/3] Fix trouble with in virsh detach-disk

[Peter Krempa]

We've created malformed  element for RBD images in backing store
of a disk which failed parsing and broke detaching disks via virsh.

Fix it by not parsing auth data at all from the disk backing images and
remove the  element when constructing the XML for disk
detach.

Peter Krempa (3):
  virsh: detach-disk: Add --print-xml switch
  util: storage: Remove detected authentication data for backing chains
  virsh: Remove  sub-element in virshFindDisk

 src/util/virstoragefile.c |  8 
 tests/virstoragetest.c| 15 +++
 tools/virsh-domain.c  | 44 +---
 tools/virsh.pod   |  4 
 4 files changed, 48 insertions(+), 23 deletions(-)

-- 
2.15.0

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

Re: [libvirt] [PATCH 3/6] spec: Drop checks for old Fedora releases

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 03:11:38PM +0100, Jiri Denemark wrote:
> The oldest Fedora release supported by the spec file is 26. Checking for
> anything older makes no sense.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 24 +---
>  1 file changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 5d05acd620..daf7098216 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -73,7 +73,7 @@
>  %define with_numactl  0%{!?_without_numactl:1}
>  
>  # F25+ has zfs-fuse
> -%if 0%{?fedora} >= 25
> +%if 0%{?fedora}
>  %define with_storage_zfs  0%{!?_without_storage_zfs:1}
>  %else
>  %define with_storage_zfs  0
> @@ -233,14 +233,10 @@
>  %define enable_werror --disable-werror
>  %endif
>  
> -%if 0%{?fedora} >= 25
> +%if 0%{?fedora}
>  %define tls_priority "@LIBVIRT,SYSTEM"
>  %else
> -%if 0%{?fedora}
> -%define tls_priority "@SYSTEM"
> -%else
> -%define tls_priority "NORMAL"
> -%endif
> +%define tls_priority "NORMAL"
>  %endif
>  
>  
> @@ -451,11 +447,7 @@ BuildRequires: numad
>  %endif
>  
>  %if %{with_wireshark}
> -%if 0%{fedora} >= 24
>  BuildRequires: wireshark-devel >= 2.1.0
> -%else
> -BuildRequires: wireshark-devel >= 1.12.1
> -%endif
>  %endif
>  
>  %if %{with_libssh}
> @@ -794,7 +786,7 @@ Requires: gzip
>  Requires: bzip2
>  Requires: lzop
>  Requires: xz
> -%if 0%{?fedora} >= 24
> +%if 0%{?fedora}
>  Requires: systemd-container
>  %endif
>  
> @@ -812,7 +804,7 @@ Group: Development/Libraries
>  Requires: libvirt-daemon = %{version}-%{release}
>  # There really is a hard cross-driver dependency here
>  Requires: libvirt-daemon-driver-network = %{version}-%{release}
> -%if 0%{?fedora} >= 24
> +%if 0%{?fedora}
>  Requires: systemd-container
>  %endif
>  
> @@ -1421,13 +1413,7 @@ rm -f 
> $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a
>  rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la
>  rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a
>  %if %{with_wireshark}
> -%if 0%{fedora} >= 24
>  rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la
> -%else
> -rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.la
> -mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \
> -  $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.so
> -%endif
>  %endif
>  
>  install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}/lib/libvirt/dnsmasq/

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 4/6] spec: Prepare for future RHEL

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 03:11:39PM +0100, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index daf7098216..c5cb0439a0 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -143,6 +143,10 @@
>  %define with_libxl 0
>  %define with_hyperv 0
>  %define with_vz 0
> +
> +%if 0%{?rhel} > 7
> +%define with_lxc 0
> +%endif
>  %endif
>  
>  # Fedora 17 / RHEL-7 are first where we use systemd. Although earlier
> @@ -295,7 +299,7 @@ BuildRequires: libtool
>  BuildRequires: /usr/bin/pod2man
>  %endif
>  BuildRequires: git
> -%if 0%{?fedora} >= 27
> +%if 0%{?fedora} >= 27 || 0%{?rhel} > 7
>  BuildRequires: perl-interpreter
>  %else
>  BuildRequires: perl
> @@ -786,7 +790,7 @@ Requires: gzip
>  Requires: bzip2
>  Requires: lzop
>  Requires: xz
> -%if 0%{?fedora}
> +%if 0%{?fedora} || 0%{?rhel} > 7
>  Requires: systemd-container
>  %endif
>  
> @@ -804,7 +808,7 @@ Group: Development/Libraries
>  Requires: libvirt-daemon = %{version}-%{release}
>  # There really is a hard cross-driver dependency here
>  Requires: libvirt-daemon-driver-network = %{version}-%{release}
> -%if 0%{?fedora}
> +%if 0%{?fedora} || 0%{?rhel} > 7
>  Requires: systemd-container
>  %endif

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 6/6] spec: Drop overlapping triggers

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 03:11:41PM +0100, Jiri Denemark wrote:
> The postun trigger for libvirt-daemon was defined twice for overlapping
> ranges of package verions if systemd support was switched off (which
> happens when building on something ancient, such as RHEL-6).
> 
> Let's combine the two triggers into the one which is called when
> libvirt-daemon < 1.3.0 is uninstalled. As a side effect, virtlockd and
> virtlogd might be reloaded twice after an upgrade from libvirt newer
> than 1.2.1 and older than 1.3.0 (by postun script from the old libvirt
> and postun trigger from the new libvirt).
> 
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 11 ++-
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index f73fcab494..e1e902c5e4 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1599,15 +1599,6 @@ if [ $1 -ge 1 ]; then
>  fi
>  %endif
>  
> -%if %{with_systemd}
> -%else
> -%triggerpostun daemon -- libvirt-daemon < 1.2.1
> -if [ "$1" -ge "1" ]; then
> -/sbin/service virtlockd reload > /dev/null 2>&1 || :
> -/sbin/service virtlogd reload > /dev/null 2>&1 || :
> -fi
> -%endif
> -
>  # In upgrade scenario we must explicitly enable virtlockd/virtlogd
>  # sockets, if libvirtd is already enabled and start them if
>  # libvirtd is running, otherwise you'll get failures to start
> @@ -1624,6 +1615,8 @@ if [ $1 -ge 1 ] ; then
>  /sbin/chkconfig virtlogd on || :
>  /sbin/service libvirtd status 1>/dev/null 2>&1 &&
>  /sbin/service virtlogd start || :
> +/sbin/service virtlockd reload > /dev/null 2>&1 || :
> +/sbin/service virtlogd reload > /dev/null 2>&1 || :
>  %endif
>  fi

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 5/6] spec: Fix indentation in daemon's triggerpostun

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 03:11:40PM +0100, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index c5cb0439a0..f73fcab494 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1615,15 +1615,15 @@ fi
>  %triggerpostun daemon -- libvirt-daemon < 1.3.0
>  if [ $1 -ge 1 ] ; then
>  %if %{with_systemd}
> -/bin/systemctl is-enabled libvirtd.service 1>/dev/null 2>&1 &&
> -/bin/systemctl enable virtlogd.socket virtlogd-admin.socket || :
> -/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 &&
> -/bin/systemctl start virtlogd.socket virtlogd-admin.socket || :
> +/bin/systemctl is-enabled libvirtd.service 1>/dev/null 2>&1 &&
> +/bin/systemctl enable virtlogd.socket virtlogd-admin.socket || :
> +/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 &&
> +/bin/systemctl start virtlogd.socket virtlogd-admin.socket || :
>  %else
> -/sbin/chkconfig libvirtd 1>/dev/null 2>&1 &&
> -/sbin/chkconfig virtlogd on || :
> -/sbin/service libvirtd status 1>/dev/null 2>&1 &&
> -/sbin/service virtlogd start || :
> +/sbin/chkconfig libvirtd 1>/dev/null 2>&1 &&
> +/sbin/chkconfig virtlogd on || :
> +/sbin/service libvirtd status 1>/dev/null 2>&1 &&
> +/sbin/service virtlogd start || :
>  %endif
>  fi

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 2/6] spec: Build virt-login-shell iff LXC driver is enabled

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 03:11:37PM +0100, Jiri Denemark wrote:
> Building virt-login-shell doesn't really make any sense without LXC and
> doing so even breaks "make rpm" since the associated files are installed
> but unpackaged (the login-shell sub package already depends on LXC).
> 
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 4821da826e..5d05acd620 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1182,8 +1182,10 @@ exit 1
>  
>  %if %{with_lxc}
>  %define arg_lxc --with-lxc
> +%define arg_login_shell --with-login-shell
>  %else
>  %define arg_lxc --without-lxc
> +%define arg_login_shell --without-login-shell
>  %endif
>  
>  %if %{with_vbox}
> @@ -1393,7 +1395,8 @@ rm -f po/stamp-po
> %{?arg_loader_nvram} \
> %{?enable_werror} \
> --enable-expensive-tests \
> -   %{arg_init_script}
> +   %{arg_init_script} \
> +   %{?arg_login_shell}
>  make %{?_smp_mflags} V=1
>  gzip -9 ChangeLog

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 1/6] spec: Enable fuse only if LXC is enabled

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 03:11:36PM +0100, Jiri Denemark wrote:
> Enabling fuse without LXC does not make a lot of sense because fuse is
> used only by LXC.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 44f846a169..4821da826e 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -163,7 +163,7 @@
>  %endif
>  
>  # fuse is used to provide virtualized /proc for LXC
> -%if 0%{?fedora} || 0%{?rhel} >= 7
> +%if %{with_lxc}
>  %define with_fuse  0%{!?_without_fuse:1}
>  %endif

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH 5/6] spec: Fix indentation in daemon's triggerpostun

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index c5cb0439a0..f73fcab494 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1615,15 +1615,15 @@ fi
 %triggerpostun daemon -- libvirt-daemon < 1.3.0
 if [ $1 -ge 1 ] ; then
 %if %{with_systemd}
-/bin/systemctl is-enabled libvirtd.service 1>/dev/null 2>&1 &&
-/bin/systemctl enable virtlogd.socket virtlogd-admin.socket || :
-/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 &&
-/bin/systemctl start virtlogd.socket virtlogd-admin.socket || :
+/bin/systemctl is-enabled libvirtd.service 1>/dev/null 2>&1 &&
+/bin/systemctl enable virtlogd.socket virtlogd-admin.socket || :
+/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 &&
+/bin/systemctl start virtlogd.socket virtlogd-admin.socket || :
 %else
-/sbin/chkconfig libvirtd 1>/dev/null 2>&1 &&
-/sbin/chkconfig virtlogd on || :
-/sbin/service libvirtd status 1>/dev/null 2>&1 &&
-/sbin/service virtlogd start || :
+/sbin/chkconfig libvirtd 1>/dev/null 2>&1 &&
+/sbin/chkconfig virtlogd on || :
+/sbin/service libvirtd status 1>/dev/null 2>&1 &&
+/sbin/service virtlogd start || :
 %endif
 fi
 
-- 
2.16.1

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

[libvirt] [PATCH 1/6] spec: Enable fuse only if LXC is enabled

[Jiri Denemark]

Enabling fuse without LXC does not make a lot of sense because fuse is
used only by LXC.

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 44f846a169..4821da826e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -163,7 +163,7 @@
 %endif
 
 # fuse is used to provide virtualized /proc for LXC
-%if 0%{?fedora} || 0%{?rhel} >= 7
+%if %{with_lxc}
 %define with_fuse  0%{!?_without_fuse:1}
 %endif
 
-- 
2.16.1

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

[libvirt] [PATCH 2/6] spec: Build virt-login-shell iff LXC driver is enabled

[Jiri Denemark]

Building virt-login-shell doesn't really make any sense without LXC and
doing so even breaks "make rpm" since the associated files are installed
but unpackaged (the login-shell sub package already depends on LXC).

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 4821da826e..5d05acd620 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1182,8 +1182,10 @@ exit 1
 
 %if %{with_lxc}
 %define arg_lxc --with-lxc
+%define arg_login_shell --with-login-shell
 %else
 %define arg_lxc --without-lxc
+%define arg_login_shell --without-login-shell
 %endif
 
 %if %{with_vbox}
@@ -1393,7 +1395,8 @@ rm -f po/stamp-po
%{?arg_loader_nvram} \
%{?enable_werror} \
--enable-expensive-tests \
-   %{arg_init_script}
+   %{arg_init_script} \
+   %{?arg_login_shell}
 make %{?_smp_mflags} V=1
 gzip -9 ChangeLog
 
-- 
2.16.1

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

[libvirt] [PATCH 6/6] spec: Drop overlapping triggers

[Jiri Denemark]

The postun trigger for libvirt-daemon was defined twice for overlapping
ranges of package verions if systemd support was switched off (which
happens when building on something ancient, such as RHEL-6).

Let's combine the two triggers into the one which is called when
libvirt-daemon < 1.3.0 is uninstalled. As a side effect, virtlockd and
virtlogd might be reloaded twice after an upgrade from libvirt newer
than 1.2.1 and older than 1.3.0 (by postun script from the old libvirt
and postun trigger from the new libvirt).

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index f73fcab494..e1e902c5e4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1599,15 +1599,6 @@ if [ $1 -ge 1 ]; then
 fi
 %endif
 
-%if %{with_systemd}
-%else
-%triggerpostun daemon -- libvirt-daemon < 1.2.1
-if [ "$1" -ge "1" ]; then
-/sbin/service virtlockd reload > /dev/null 2>&1 || :
-/sbin/service virtlogd reload > /dev/null 2>&1 || :
-fi
-%endif
-
 # In upgrade scenario we must explicitly enable virtlockd/virtlogd
 # sockets, if libvirtd is already enabled and start them if
 # libvirtd is running, otherwise you'll get failures to start
@@ -1624,6 +1615,8 @@ if [ $1 -ge 1 ] ; then
 /sbin/chkconfig virtlogd on || :
 /sbin/service libvirtd status 1>/dev/null 2>&1 &&
 /sbin/service virtlogd start || :
+/sbin/service virtlockd reload > /dev/null 2>&1 || :
+/sbin/service virtlogd reload > /dev/null 2>&1 || :
 %endif
 fi
 
-- 
2.16.1

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

[libvirt] [PATCH 3/6] spec: Drop checks for old Fedora releases

[Jiri Denemark]

The oldest Fedora release supported by the spec file is 26. Checking for
anything older makes no sense.

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 24 +---
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 5d05acd620..daf7098216 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -73,7 +73,7 @@
 %define with_numactl  0%{!?_without_numactl:1}
 
 # F25+ has zfs-fuse
-%if 0%{?fedora} >= 25
+%if 0%{?fedora}
 %define with_storage_zfs  0%{!?_without_storage_zfs:1}
 %else
 %define with_storage_zfs  0
@@ -233,14 +233,10 @@
 %define enable_werror --disable-werror
 %endif
 
-%if 0%{?fedora} >= 25
+%if 0%{?fedora}
 %define tls_priority "@LIBVIRT,SYSTEM"
 %else
-%if 0%{?fedora}
-%define tls_priority "@SYSTEM"
-%else
-%define tls_priority "NORMAL"
-%endif
+%define tls_priority "NORMAL"
 %endif
 
 
@@ -451,11 +447,7 @@ BuildRequires: numad
 %endif
 
 %if %{with_wireshark}
-%if 0%{fedora} >= 24
 BuildRequires: wireshark-devel >= 2.1.0
-%else
-BuildRequires: wireshark-devel >= 1.12.1
-%endif
 %endif
 
 %if %{with_libssh}
@@ -794,7 +786,7 @@ Requires: gzip
 Requires: bzip2
 Requires: lzop
 Requires: xz
-%if 0%{?fedora} >= 24
+%if 0%{?fedora}
 Requires: systemd-container
 %endif
 
@@ -812,7 +804,7 @@ Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # There really is a hard cross-driver dependency here
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
-%if 0%{?fedora} >= 24
+%if 0%{?fedora}
 Requires: systemd-container
 %endif
 
@@ -1421,13 +1413,7 @@ rm -f 
$RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a
 rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la
 rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a
 %if %{with_wireshark}
-%if 0%{fedora} >= 24
 rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la
-%else
-rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.la
-mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \
-  $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.so
-%endif
 %endif
 
 install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}/lib/libvirt/dnsmasq/
-- 
2.16.1

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

[libvirt] [PATCH 0/6] spec: Misc cleanups and improvements

[Jiri Denemark]

Jiri Denemark (6):
  spec: Enable fuse only if LXC is enabled
  spec: Build virt-login-shell iff LXC driver is enabled
  spec: Drop checks for old Fedora releases
  spec: Prepare for future RHEL
  spec: Fix indentation in daemon's triggerpostun
  spec: Drop overlapping triggers

 libvirt.spec.in | 64 ++---
 1 file changed, 25 insertions(+), 39 deletions(-)

-- 
2.16.1

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

[libvirt] [PATCH 4/6] spec: Prepare for future RHEL

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index daf7098216..c5cb0439a0 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -143,6 +143,10 @@
 %define with_libxl 0
 %define with_hyperv 0
 %define with_vz 0
+
+%if 0%{?rhel} > 7
+%define with_lxc 0
+%endif
 %endif
 
 # Fedora 17 / RHEL-7 are first where we use systemd. Although earlier
@@ -295,7 +299,7 @@ BuildRequires: libtool
 BuildRequires: /usr/bin/pod2man
 %endif
 BuildRequires: git
-%if 0%{?fedora} >= 27
+%if 0%{?fedora} >= 27 || 0%{?rhel} > 7
 BuildRequires: perl-interpreter
 %else
 BuildRequires: perl
@@ -786,7 +790,7 @@ Requires: gzip
 Requires: bzip2
 Requires: lzop
 Requires: xz
-%if 0%{?fedora}
+%if 0%{?fedora} || 0%{?rhel} > 7
 Requires: systemd-container
 %endif
 
@@ -804,7 +808,7 @@ Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # There really is a hard cross-driver dependency here
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
-%if 0%{?fedora}
+%if 0%{?fedora} || 0%{?rhel} > 7
 Requires: systemd-container
 %endif
 
-- 
2.16.1

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

Re: [libvirt] [PATCH 0/3] virSysinfo: Introduce SMBIOS type 3 support

[John Ferlan]

On 02/14/2018 04:22 AM, Zhuangyanying wrote:
> From: Zhuang Yanying 
> 
> Some applications inside VM need to access SMBIOS Chassis Asset Tag,
> which should be emulated.
> 
> access inside VM (for example)
> Linux:   /sys/class/dmi/id/chassis_asset_tag.
> Windows: (Get-WmiObject Win32_SystemEnclosure).SMBIOSAssetTag
>   wirhin Windows PowerShell.
> 
> It has already been realized in qemu:
> 
> SMBIOS: Build aggregate smbios tables and entry point
> https://git.qemu.org/?p=qemu.git;a=commit;h=c97294ec1b9e36887e119589d456557d72ab37b5
> 
> but not in libvirt. we realize it here.
> As an example, you could use something like
> 
> 
>   Huawei
>   To be filled by O.E.M.
>   To be filled by O.E.M.
>   To be filled by O.E.M.

Would prefer some more "realistic values" rather than "To be filled by
O.E.M."...  They don't have to be exactly what is on your system, but
closer to expectations would be nice.  Similar to what already exists.

You can just respond here and I can make the changes for you.

NB: The xml files you put in patch2 should have been in patch1 - I can
move those too.

Other than that everything looks good to me.

John

>   Type3Sku1
> 
> 
> BTW: I'll be on vacation for china spring festival for the next week, I'll 
> response as soon as I get back if there's any modification needed.
> 
> Zhuang Yanying (3):
>   conf: add support for setting Chassis SMBIOS data fields
>   qemu: add support for generating SMBIOS Chassis strings command line
>   news: add support for setting Chassis SMBIOS data fields
> 
>  docs/formatdomain.html.in   |  23 +++
>  docs/news.xml   |   5 ++
>  docs/schemas/domaincommon.rng   |  22 ++
>  src/conf/domain_conf.c  |  55 +++
>  src/libvirt_private.syms|   1 +
>  src/qemu/qemu_command.c |  51 ++
>  src/util/virsysinfo.c   | 133 
> +++-
>  src/util/virsysinfo.h   |  13 
>  tests/qemuxml2argvdata/smbios.args  |   2 +
>  tests/qemuxml2argvdata/smbios.xml   |   7 ++
>  tests/qemuxml2xmloutdata/smbios.xml |   7 ++
>  11 files changed, 318 insertions(+), 1 deletion(-)
> 

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

Re: [libvirt] [PATCH v2 0/2] Fix parsing and formatting of 'UnixSocketAddress' qapi type

[Ján Tomko]

On Wed, Feb 14, 2018 at 01:33:09PM +0100, Peter Krempa wrote:

v2: Don't 'fix' it for gluster disks which use old syntax not conforming
to the schema in qemu.

Peter Krempa (2):
 storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'
 virstoragetest: Add test case for NBD over unix socket with new syntax

src/qemu/qemu_block.c | 13 +++--
src/util/virstoragefile.c |  8 +++-
tests/virstoragetest.c|  9 +
3 files changed, 27 insertions(+), 3 deletions(-)



ACK series

Jan


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

[libvirt] [PATCH 0/3] virSysinfo: Introduce SMBIOS type 3 support

[Zhuangyanying]

From: Zhuang Yanying 

Some applications inside VM need to access SMBIOS Chassis Asset Tag,
which should be emulated.

access inside VM (for example)
Linux:   /sys/class/dmi/id/chassis_asset_tag.
Windows: (Get-WmiObject Win32_SystemEnclosure).SMBIOSAssetTag
  wirhin Windows PowerShell.

It has already been realized in qemu:

SMBIOS: Build aggregate smbios tables and entry point
https://git.qemu.org/?p=qemu.git;a=commit;h=c97294ec1b9e36887e119589d456557d72ab37b5

but not in libvirt. we realize it here.
As an example, you could use something like


  Huawei
  To be filled by O.E.M.
  To be filled by O.E.M.
  To be filled by O.E.M.
  Type3Sku1


BTW: I'll be on vacation for china spring festival for the next week, I'll 
response as soon as I get back if there's any modification needed.

Zhuang Yanying (3):
  conf: add support for setting Chassis SMBIOS data fields
  qemu: add support for generating SMBIOS Chassis strings command line
  news: add support for setting Chassis SMBIOS data fields

 docs/formatdomain.html.in   |  23 +++
 docs/news.xml   |   5 ++
 docs/schemas/domaincommon.rng   |  22 ++
 src/conf/domain_conf.c  |  55 +++
 src/libvirt_private.syms|   1 +
 src/qemu/qemu_command.c |  51 ++
 src/util/virsysinfo.c   | 133 +++-
 src/util/virsysinfo.h   |  13 
 tests/qemuxml2argvdata/smbios.args  |   2 +
 tests/qemuxml2argvdata/smbios.xml   |   7 ++
 tests/qemuxml2xmloutdata/smbios.xml |   7 ++
 11 files changed, 318 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 2/3] qemu: add support for generating SMBIOS Chassis strings command line

[Zhuangyanying]

From: Zhuang Yanying 

This wires up the previously added Chassis strings XML schema to be able to
generate comamnd line args for QEMU. This requires QEMU >= 2.1 release
containing this patch:

SMBIOS: Build aggregate smbios tables and entry point
https://git.qemu.org/?p=qemu.git;a=commit;h=c97294ec1b9e36887e119589d456557d72ab37b5

Signed-off-by: Zhuang Yanying 
---
 src/qemu/qemu_command.c | 51 +
 tests/qemuxml2argvdata/smbios.args  |  2 ++
 tests/qemuxml2argvdata/smbios.xml   |  7 +
 tests/qemuxml2xmloutdata/smbios.xml |  7 +
 4 files changed, 67 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c73cd7..266b354 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5817,6 +5817,51 @@ qemuBuildSmbiosOEMStringsStr(virSysinfoOEMStringsDefPtr 
def)
 }
 
 
+static char *
+qemuBuildSmbiosChassisStr(virSysinfoChassisDefPtr def)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+if (!def)
+return NULL;
+
+virBufferAddLit(, "type=3");
+
+/* 3:Manufacturer */
+virBufferAddLit(, ",manufacturer=");
+virQEMUBuildBufferEscapeComma(, def->manufacturer);
+/* 3:Version */
+if (def->version) {
+virBufferAddLit(, ",version=");
+virQEMUBuildBufferEscapeComma(, def->version);
+}
+/* 3:Serial Number */
+if (def->serial) {
+virBufferAddLit(, ",serial=");
+virQEMUBuildBufferEscapeComma(, def->serial);
+}
+/* 3:Asset Tag */
+if (def->asset) {
+virBufferAddLit(, ",asset=");
+virQEMUBuildBufferEscapeComma(, def->asset);
+}
+/* 3:Sku */
+if (def->sku) {
+virBufferAddLit(, ",sku=");
+virQEMUBuildBufferEscapeComma(, def->sku);
+}
+
+if (virBufferCheckError() < 0)
+goto error;
+
+return virBufferContentAndReset();
+
+ error:
+virBufferFreeAndReset();
+return NULL;
+}
+
+
 static int
 qemuBuildSmbiosCommandLine(virCommandPtr cmd,
virQEMUDriverPtr driver,
@@ -5888,6 +5933,12 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
 VIR_FREE(smbioscmd);
 }
 
+smbioscmd = qemuBuildSmbiosChassisStr(source->chassis);
+if (smbioscmd != NULL) {
+virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
+VIR_FREE(smbioscmd);
+}
+
 if (source->oemStrings) {
 if (!(smbioscmd = 
qemuBuildSmbiosOEMStringsStr(source->oemStrings)))
 return -1;
diff --git a/tests/qemuxml2argvdata/smbios.args 
b/tests/qemuxml2argvdata/smbios.args
index d27d436..2f0a89f 100644
--- a/tests/qemuxml2argvdata/smbios.args
+++ b/tests/qemuxml2argvdata/smbios.args
@@ -17,6 +17,8 @@ serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\
 uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \
 -smbios 'type=2,manufacturer=Hewlett-Packard,product=0B4Ch,version=D,\
 serial=CZC1065993,asset=CZC1065993,location=Upside down' \
+-smbios 'type=3,manufacturer=Huawei,version=To be filled by O.E.M.,\
+serial=To be filled by O.E.M.,asset=To be filled by O.E.M.,sku=Type3Sku1' \
 -smbios 'type=11,value=Hello,value=World,value=This is,,\
  more tricky value=escaped' \
 -nographic \
diff --git a/tests/qemuxml2argvdata/smbios.xml 
b/tests/qemuxml2argvdata/smbios.xml
index 319bdf6..474b7d8 100644
--- a/tests/qemuxml2argvdata/smbios.xml
+++ b/tests/qemuxml2argvdata/smbios.xml
@@ -26,6 +26,13 @@
   CZC1065993
   Upside down
 
+
+  Huawei
+  To be filled by O.E.M.
+  To be filled by O.E.M.
+  To be filled by O.E.M.
+  Type3Sku1
+
 
   Hello
   World
diff --git a/tests/qemuxml2xmloutdata/smbios.xml 
b/tests/qemuxml2xmloutdata/smbios.xml
index cbe616c..5ef9402 100644
--- a/tests/qemuxml2xmloutdata/smbios.xml
+++ b/tests/qemuxml2xmloutdata/smbios.xml
@@ -26,6 +26,13 @@
   CZC1065993
   Upside down
 
+
+  Huawei
+  To be filled by O.E.M.
+  To be filled by O.E.M.
+  To be filled by O.E.M.
+  Type3Sku1
+
 
   Hello
   World
-- 
1.8.3.1


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

[libvirt] [PATCH v2 1/2] storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'

[Peter Krempa]

The documentation for the JSON/qapi type 'UnixSocketAddress' states that
the unix socket path field is named 'path'. Unfortunately qemu uses
'socket' in case of the gluster driver (despite documented otherwise).

Add logic which will format the correct fields while keeping support of
the old spelling.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1544325

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 13 +++--
 src/util/virstoragefile.c |  8 +++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 8b71679118..6f81e9d96c 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -467,11 +467,14 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src)
 /**
  * qemuBlockStorageSourceBuildJSONSocketAddress
  * @host: the virStorageNetHostDefPtr definition to build
- * @legacy: use 'tcp' instead of 'inet' for compatibility reasons
+ * @legacy: use old field names/values
  *
  * Formats @hosts into a json object conforming to the 'SocketAddress' type
  * in qemu.
  *
+ * For compatibility with old approach used in the gluster driver of old qemus
+ * use the old spelling for TCP transport and, the path field of the unix 
socket.
+ *
  * Returns a virJSONValuePtr for a single server.
  */
 static virJSONValuePtr
@@ -481,6 +484,7 @@ 
qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host,
 virJSONValuePtr server = NULL;
 virJSONValuePtr ret = NULL;
 const char *transport;
+const char *field;
 char *port = NULL;

 switch ((virStorageNetHostTransport) host->transport) {
@@ -502,9 +506,14 @@ 
qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host,
 break;

 case VIR_STORAGE_NET_HOST_TRANS_UNIX:
+if (legacy)
+field = "s:socket";
+else
+field = "s:path";
+
 if (virJSONValueObjectCreate(,
  "s:type", "unix",
- "s:socket", host->socket,
+ field, host->socket,
  NULL) < 0)
 goto cleanup;
 break;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 7f878039ba..ebfb0a860a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2893,7 +2893,13 @@ 
virStorageSourceParseBackingJSONSocketAddress(virStorageNetHostDefPtr host,
 } else if (STREQ(type, "unix")) {
 host->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX;

-if (!(socket = virJSONValueObjectGetString(json, "socket"))) {
+socket = virJSONValueObjectGetString(json, "path");
+
+/* check for old spelling for gluster protocol */
+if (!socket)
+socket = virJSONValueObjectGetString(json, "socket");
+
+if (!socket) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("missing socket path for udp backing server in "
  "JSON backing volume definition"));
-- 
2.15.0

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

[libvirt] [PATCH v2 0/2] Fix parsing and formatting of 'UnixSocketAddress' qapi type

[Peter Krempa]

v2: Don't 'fix' it for gluster disks which use old syntax not conforming
to the schema in qemu.

Peter Krempa (2):
  storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'
  virstoragetest: Add test case for NBD over unix socket with new syntax

 src/qemu/qemu_block.c | 13 +++--
 src/util/virstoragefile.c |  8 +++-
 tests/virstoragetest.c|  9 +
 3 files changed, 27 insertions(+), 3 deletions(-)

-- 
2.15.0

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

[libvirt] [PATCH v2 2/2] virstoragetest: Add test case for NBD over unix socket with new syntax

[Peter Krempa]

Use the new syntax which uses the 'UnixSocket' type in qemu.

Signed-off-by: Peter Krempa 
---
 tests/virstoragetest.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 6eed7134ed..16c271c781 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1494,6 +1494,15 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\","
+   "\"server\": { \"type\":\"unix\","
+ 
"\"path\":\"/path/socket\""
+   "}"
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
 TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"ssh\","
"\"host\":\"example.org\","
"\"port\":\"6000\","
-- 
2.15.0

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

Re: [libvirt] [jenkins-ci PATCH] jobs: Shorten name for python-distutils jobs

[Daniel P . Berrangé]

On Wed, Feb 14, 2018 at 12:47:04PM +0100, Andrea Bolognani wrote:
> Instead of passing the full name of the Python binary as 'python',
> only pass the major version as 'pyver' and build everything else
> based on that.
> 
> Doing so allows us to make a few things, most notably job names,
> slightly shorter and nicer.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> I wouldn't bother changing this if the previous change to Python
> jobs had already been deployed, but since it hasn't yet might as
> well have nicer names instead :)
> 
>  jobs/python-distutils.yaml   | 22 +++---
>  projects/libvirt-python.yaml | 16 
>  projects/virt-manager.yaml   | 14 +++---
>  3 files changed, 26 insertions(+), 26 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [jenkins-ci PATCH] jobs: Shorten name for python-distutils jobs

[Andrea Bolognani]

Instead of passing the full name of the Python binary as 'python',
only pass the major version as 'pyver' and build everything else
based on that.

Doing so allows us to make a few things, most notably job names,
slightly shorter and nicer.

Signed-off-by: Andrea Bolognani 
---
I wouldn't bother changing this if the previous change to Python
jobs had already been deployed, but since it hasn't yet might as
well have nicer names instead :)

 jobs/python-distutils.yaml   | 22 +++---
 projects/libvirt-python.yaml | 16 
 projects/virt-manager.yaml   | 14 +++---
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
index 122c759..ff68c29 100644
--- a/jobs/python-distutils.yaml
+++ b/jobs/python-distutils.yaml
@@ -1,11 +1,11 @@
 
 - job-template:
 id: python-distutils-build-job
-name: '{name}-{branch}-{python}-build'
+name: '{name}-{branch}-py{pyver}-build'
 project-type: matrix
-description: '{title} Build ({python})'
+description: '{title} Build (Python {pyver})'
 command_pre_build: ''
-workspace: '{name}-{branch}-{python}'
+workspace: '{name}-{branch}-py{pyver}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -43,8 +43,8 @@
   {global_env}
   {local_env}
   {command_pre_build}
-  {python} ./setup.py build
-  {python} ./setup.py install --prefix=$VIRT_PREFIX
+  python{pyver} ./setup.py build
+  python{pyver} ./setup.py install --prefix=$VIRT_PREFIX
 publishers:
   - email:
   recipients: '{obj:spam}'
@@ -54,9 +54,9 @@
 
 - job-template:
 id: python-distutils-check-job
-name: '{name}-{branch}-{python}-check'
+name: '{name}-{branch}-py{pyver}-check'
 project-type: matrix
-description: '{title} Check ({python})'
+description: '{title} Check (Python {pyver})'
 workspace: '{name}-{branch}'
 child-workspace: '.'
 block-downstream: true
@@ -83,7 +83,7 @@
   - shell: |
   {global_env}
   {local_env}
-  {python} ./setup.py test
+  python{pyver} ./setup.py test
 publishers:
   - email:
   recipients: '{obj:spam}'
@@ -92,9 +92,9 @@
 
 - job-template:
 id: python-distutils-rpm-job
-name: '{name}-{branch}-{python}-rpm'
+name: '{name}-{branch}-py{pyver}-rpm'
 project-type: matrix
-description: '{title} RPM ({python})'
+description: '{title} RPM (Python {pyver})'
 workspace: '{name}-{branch}'
 child-workspace: '.'
 block-downstream: true
@@ -122,7 +122,7 @@
   {global_env}
   {local_env}
   sed -i -e 's/BuildRequires: libvirt.*devel.*//' *.spec.in
-  {python} ./setup.py rpm
+  python{pyver} ./setup.py rpm
 publishers:
   - email:
   recipients: '{obj:spam}'
diff --git a/projects/libvirt-python.yaml b/projects/libvirt-python.yaml
index e065c5d..1c29321 100644
--- a/projects/libvirt-python.yaml
+++ b/projects/libvirt-python.yaml
@@ -14,10 +14,10 @@
 title: Libvirt Python
 jobs:
   - python-distutils-build-job:
-  python: python2
+  pyver: 2
   parent_jobs: 'libvirt-master-build'
   - python-distutils-build-job:
-  python: python3
+  pyver: 3
   parent_jobs: 'libvirt-master-build'
   machines:
 - libvirt-debian-8
@@ -28,11 +28,11 @@
 - libvirt-freebsd-10
 - libvirt-freebsd-11
   - python-distutils-check-job:
-  python: python2
-  parent_jobs: 'libvirt-python-master-{python}-build'
+  pyver: 2
+  parent_jobs: 'libvirt-python-master-py{pyver}-build'
   - python-distutils-check-job:
-  python: python3
-  parent_jobs: 'libvirt-python-master-{python}-build'
+  pyver: 3
+  parent_jobs: 'libvirt-python-master-py{pyver}-build'
   machines:
 - libvirt-debian-8
 - libvirt-debian-9
@@ -42,8 +42,8 @@
 - libvirt-freebsd-10
 - libvirt-freebsd-11
   - python-distutils-rpm-job:
-  python: python2
-  parent_jobs: 'libvirt-python-master-{python}-check'
+  pyver: 2
+  parent_jobs: 'libvirt-python-master-py{pyver}-check'
   machines:
 - libvirt-centos-6
 - libvirt-centos-7
diff --git a/projects/virt-manager.yaml b/projects/virt-manager.yaml
index b5b0df5..8f3112e 100644
--- a/projects/virt-manager.yaml
+++ b/projects/virt-manager.yaml
@@ -11,18 +11,18 @@
 title: Virtual Machine Manager
 jobs:
   - python-distutils-build-job:
-  python: python3
+  pyver: 3
   parent_jobs:
-- 'libvirt-python-master-{python}-build'
+- 'libvirt-python-master-py{pyver}-build'
 - 'libosinfo-master-build'
   command_pre_build: |
-{python} 

Re: [libvirt] [RFC] cgroup settings and systemd daemon-reload conflict

[Daniel P . Berrangé]

On Tue, Jan 30, 2018 at 10:34:14AM +0300, Nikolay Shirokovskiy wrote:
> Hi, all.
> 
> It turns out that systemd daemon-reload reset settings that are managable
> thru 'systemctl set-property' interface.
> 
> > virsh schedinfo tst3  | grep global_quota
> global_quota   : -1
> > virsh schedinfo tst3 --set global_quota=5 | grep global_quota
> global_quota   : 5
> > systemctl daemon-reload
> > virsh schedinfo tst3  | grep global_quota
> global_quota   : -1
> 
> This behaviour does not limited to cpu controller, same for blkio for example.
> I checked different versions of systemd (219 - Feb 15, and quite recent 236 - 
> Dec 17)
> to make sure it is not kind of bug of old version. So systemd does not play 
> well
> with direct writes to cgroup parameters that managable thru systemd. Looks 
> like
> libvirtd needs to use systemd's dbus interface to change all such parameters.
> 
> I only wonder how this can be unnoticed for such long time (creating cgroup 
> for domain
> thru systemd - Jul 2013) as daemon-reload is called upon libvirtd package 
> update. May
> be I miss something?

I guess the reasons its unnoticed is that using global_* constants is
relatively uncommon. Traditionally we had the per-vCPU tunables for
quota and the global stuff was a newer addition.  Also for a long
time I didn't think systemd even supported the quota+period settings
in unit files at all.

It is incredibly frustrating that systemd would just reset th cgroups
settings on daemon reload - something which ostensibly is supposed to
be used to reload config files on disk. Since this behaviour has existed
a long time though, I guess we have little choice but to cope with it
now.

We kind of need to use the systemd dbus API more in order to support
cgroups v2 properly too.

None of this will be pleasant changes to make though as this area of
code is fairly complex. Also I don't think there's any nice way to
introspect what properties a given version of systemd supports, which
makes it hard to know when to set direct vs when to set via dbus. I
think we would have to create the machine via dbus, then read back the
auto-generated unit file for the machine to see what properties are
listed as existing, then we can see which we now need to set via dbus.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH 1/3] conf: add support for setting Chassis SMBIOS data fields

[Zhuangyanying]

From: Zhuang Yanying 

This type of information defines attributes of a system
chassis, such as SMBIOS Chassis Asset Tag.

access inside VM (for example)
Linux:   /sys/class/dmi/id/chassis_asset_tag.
Windows: (Get-WmiObject Win32_SystemEnclosure).SMBIOSAssetTag
  wirhin Windows PowerShell.

As an example, you could use something like


  Huawei
  To be filled by O.E.M.
  To be filled by O.E.M.
  To be filled by O.E.M.
  Type3Sku1


Signed-off-by: Zhuang Yanying 
---
 docs/formatdomain.html.in |  23 
 docs/schemas/domaincommon.rng |  22 +++
 src/conf/domain_conf.c|  55 +
 src/libvirt_private.syms  |   1 +
 src/util/virsysinfo.c | 133 +-
 src/util/virsysinfo.h |  13 +
 6 files changed, 246 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3ec1173..bcbf6fd 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -411,6 +411,13 @@
 entry name='version'0B98401 Pro/entry
 entry name='serial'W1KS427111E/entry
   /baseBoard
+  chassis
+entry name='manufacturer'Huawei/entry
+entry name='version'To be filled by O.E.M./entry
+entry name='serial'To be filled by O.E.M./entry
+entry name='asset'To be filled by O.E.M./entry
+entry name='sku'Type3Sku1/entry
+  /chassis
   oemStrings
 entrymyappname:some arbitrary data/entry
 entryotherappname:more arbitrary data/entry
@@ -502,6 +509,22 @@
 validation and date format checking, all values are
 passed as strings to the hypervisor driver.
   
+  chassis
+  
+This is block 3 of SMBIOS, with entry names drawn from:
+
+manufacturer
+Manufacturer of Chassis
+version
+Version of the Chassis
+serial
+Serial number
+asset
+Asset tag
+sku
+SKU number
+
+  
   oemStrings
   
 This is block 11 of SMBIOS. This element should appear once and
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ee6f83c..8165e69 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4896,6 +4896,18 @@
   
 
 
+  
+
+  
+
+  
+
+
+  
+
+  
+
+
   
 
   
@@ -4940,6 +4952,16 @@
 
   
 
+  
+
+  manufacturer
+  version
+  serial
+  asset
+  sku
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fb732a0..e369235 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14542,6 +14542,50 @@ virSysinfoOEMStringsParseXML(xmlXPathContextPtr ctxt,
 return ret;
 }
 
+
+static int
+virSysinfoChassisParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virSysinfoChassisDefPtr *chassisdef)
+{
+int ret = -1;
+virSysinfoChassisDefPtr def;
+
+if (!xmlStrEqual(node->name, BAD_CAST "chassis")) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("XML does not contain expected 'chassis' element"));
+return ret;
+}
+
+if (VIR_ALLOC(def) < 0)
+goto cleanup;
+
+def->manufacturer =
+virXPathString("string(entry[@name='manufacturer'])", ctxt);
+def->version =
+virXPathString("string(entry[@name='version'])", ctxt);
+def->serial =
+virXPathString("string(entry[@name='serial'])", ctxt);
+def->asset =
+virXPathString("string(entry[@name='asset'])", ctxt);
+def->sku =
+virXPathString("string(entry[@name='sku'])", ctxt);
+
+if (!def->manufacturer && !def->version &&
+!def->serial && !def->asset && !def->sku) {
+virSysinfoChassisDefFree(def);
+def = NULL;
+}
+
+*chassisdef = def;
+def = NULL;
+ret = 0;
+ cleanup:
+virSysinfoChassisDefFree(def);
+return ret;
+}
+
+
 static virSysinfoDefPtr
 virSysinfoParseXML(xmlNodePtr node,
   xmlXPathContextPtr ctxt,
@@ -14600,6 +14644,17 @@ virSysinfoParseXML(xmlNodePtr node,
 if (virSysinfoBaseBoardParseXML(ctxt, >baseBoard, >nbaseBoard) < 
0)
 goto error;
 
+/* Extract chassis related metadata */
+if ((tmpnode = virXPathNode("./chassis[1]", ctxt)) != NULL) {
+oldnode = ctxt->node;
+ctxt->node = tmpnode;
+if (virSysinfoChassisParseXML(tmpnode, ctxt, >chassis) < 0) {
+ctxt->node = oldnode;
+goto error;
+}
+ctxt->node = oldnode;
+}
+
 /* Extract system related metadata 

[libvirt] [PATCH 3/3] news: add support for setting Chassis SMBIOS data fields

[Zhuangyanying]

From: Zhuang Yanying 

Signed-off-by: Zhuang Yanying 
---
 docs/news.xml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 5a2943a..b60cb2d 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -72,6 +72,11 @@
   completion data.
 
   
+  
+
+  conf: add support for setting Chassis SMBIOS data fields
+
+  
 
 
 
-- 
1.8.3.1


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

Re: [libvirt] [PATCH 1/3] storage: Fix formatting and parsing of qemu type 'UnixSocketAddress'

[Peter Krempa]

On Tue, Feb 13, 2018 at 17:33:00 +0100, Ján Tomko wrote:
> On Mon, Feb 12, 2018 at 04:20:58PM +0100, Peter Krempa wrote:
> > The documentation for the JSON/qapi type 'UnixSocketAddress' states that
> > the unix socket path field is named 'path'. We used 'socket' by
> > mistake. Fix both the formatter and parser and test suite.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1544325
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> > src/qemu/qemu_block.c | 2 +-
> > src/util/virstoragefile.c | 2 +-
> > tests/virstoragetest.c| 4 ++--
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> 
> ACK with this squashed in:
> --- a/tests/qemuxml2argvdata/disk-drive-network-gluster.args
> +++ b/tests/qemuxml2argvdata/disk-drive-network-gluster.args
> @@ -30,7 +30,7 @@ id=virtio-disk1 \
> -drive file.driver=gluster,file.volume=Volume3,file.path=Image.qcow2,\
> file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\
> file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\
> -file.server.2.type=unix,file.server.2.socket=/path/to/sock,file.debug=4,\
> +file.server.2.type=unix,file.server.2.path=/path/to/sock,file.debug=4,\
> format=qcow2,if=none,id=drive-virtio-disk2 \
> -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\
> id=virtio-disk2

This prompted me to have another look down the qemu rabbit hole on why
we actually formatted this as 'socket' since the archeology expedition
into qapi didn't ever show usage of 'socket'.

It seems that gluster _only_ accepts 'socket' rather than 'path'
contrary to the documented type.

I'll post a V2 and complain loudly at qemu.


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

Re: [libvirt] [PATCHv2 1/1] include: function parameter names same in declaration

[Daniel P . Berrangé]

On Tue, Feb 13, 2018 at 01:56:59PM -0500, Christopher Venteicher wrote:
> 
> 
> - Original Message -
> > From: "Bjoern Walk" 
> > To: "Daniel P. Berrangé" 
> > Cc: "Chris Venteicher" , libvir-list@redhat.com
> > Sent: Tuesday, February 13, 2018 5:03:40 AM
> > Subject: Re: [libvirt] [PATCHv2 1/1] include: function parameter names same 
> > in declaration
> > 
> > Daniel P. Berrangé  [2018-02-13, 09:47AM +]:
> > > On Mon, Feb 12, 2018 at 03:24:03PM -0600, Chris Venteicher wrote:
> > > > Headers use same function parameter names as definition code.
> > > > 
> > > > In some cases in libvirt-domain and libvirt-network an
> > > > established
> > > > naming pattern in the header files was more consistent and
> > > > informative
> > > > in which case the implementation was modified in the c file.
> > > 
> > > > @@ -1626,11 +1626,11 @@ int
> > > > virDomainInterfaceStats (virDomainPtr dom,
> > > >   */
> > > >  # define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst"
> > > >  
> > > > -int virDomainSetInterfaceParameters
> > > > (virDomainPtr dom,
> > > > +int virDomainSetInterfaceParameters
> > > > (virDomainPtr domain,
> > > 
> > > H, I kind of expected that  "dom" would be more popular than
> > > "domain",
> > > but I see the results are somewhat contradictory.
> > > 
> > > If we just consider the header file
> > > 
> > > $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h  |
> > > wc -l
> > > 167
> > > $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h  |
> > > grep "virDomainPtr domain" | wc -l
> > > 99
> > > 
> > > So dom==68, domain=99  => 2:3
> > > 
> > > But if we consider the source as a whole
> > > 
> > > $ git grep "virDomainPtr dom"  | wc -l
> > > 1863
> > > $ git grep "virDomainPtr dom"  | grep "virDomainPtr domain" | wc -l
> > > 675
> > > 
> > > So dom=1188 domain=675  => 2:1
> > 
> > This is biased because for a temporary variable an abbreviated name
> > 'dom' is preferable, especially if you have an argument 'domain'.
> > 
> > Since function declarations serve some kind of documentation purpose,
> > I'd prefer the full name 'domain'. It reads so much better in my
> > opinion
> > and characters are cheap.
> > 
> 
> A little more background ...
> 
> I have only submitted the changes for include/libvirt/*.h so far.
> At the bottom is a list of the files impacted by a total of 286 changes 
> suggested by the clang-tidy filter.
> 
> 
> The focus here is only the horizontal relationship establishing consistency 
> between definition and declaration,
>   not the vertical line relationship of consistency between function 
> definitions. 
> 
>   function_definition_1(p1, p2, p3) ---> function_declaration_1(p1,p2,p3)
> |
> |
> |
>   function_definition_2(p1, p2, p3) ---> function_declaration_2(p1,p2,p3)
> |
> |
> | 
>   function_definition_n(p1, p2, p3) ---> function_declaration_n(p1,p2,p3)
> 
> 
> My guess is > 90 percent of the 286 changes make the declaration better.
> 
> Probably < 10 percent of the 286 changes make the declaration slightly less 
> readable (ex. domain->dom).
> 
> All of the changes make the declarations consistent with the definitions.
> 
> None of the changes make the function_definitions worse.

Ultimately this is all just code churn for no functional benefit. So if we
are going to go through this churn, I'd like to see the benefit maximised.
To me having consistent name for each common data type is more important,
so I'd like to see that be the focus, rather than just blindly applying
the results of clang-tidy.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCHv2 1/1] include: function parameter names same in declaration

[Daniel P . Berrangé]

On Tue, Feb 13, 2018 at 12:03:40PM +0100, Bjoern Walk wrote:
> Daniel P. Berrangé  [2018-02-13, 09:47AM +]:
> > On Mon, Feb 12, 2018 at 03:24:03PM -0600, Chris Venteicher wrote:
> > > Headers use same function parameter names as definition code.
> > > 
> > > In some cases in libvirt-domain and libvirt-network an established
> > > naming pattern in the header files was more consistent and informative
> > > in which case the implementation was modified in the c file.
> > 
> > > @@ -1626,11 +1626,11 @@ int virDomainInterfaceStats 
> > > (virDomainPtr dom,
> > >   */
> > >  # define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst"
> > >  
> > > -int virDomainSetInterfaceParameters (virDomainPtr 
> > > dom,
> > > +int virDomainSetInterfaceParameters (virDomainPtr 
> > > domain,
> > 
> > H, I kind of expected that  "dom" would be more popular than "domain",
> > but I see the results are somewhat contradictory.
> > 
> > If we just consider the header file
> > 
> > $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h  | wc -l
> > 167
> > $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h  | grep 
> > "virDomainPtr domain" | wc -l
> > 99
> > 
> > So dom==68, domain=99  => 2:3
> > 
> > But if we consider the source as a whole
> > 
> > $ git grep "virDomainPtr dom"  | wc -l
> > 1863
> > $ git grep "virDomainPtr dom"  | grep "virDomainPtr domain" | wc -l
> > 675
> > 
> > So dom=1188 domain=675  => 2:1
> 
> This is biased because for a temporary variable an abbreviated name
> 'dom' is preferable, especially if you have an argument 'domain'.

That is a situation we should try to avoid as remembering which type
is which for dom vs domain is painful. We generally want to aim for

  virDomainPtr  dom
  virDomainObjPtr obj or vm
  
> Since function declarations serve some kind of documentation purpose,
> I'd prefer the full name 'domain'. It reads so much better in my opinion
> and characters are cheap.

"domain" is unneccessarily verbose.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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