[libvirt] [RFC] How to build libvirt package with git commit info from libvirt.git
Hello there, Currently I am searching a way to build a libvirt package from latest libvirt.git with package name including git commit info. Like # cd libvirt && git describe v1.2.14-rc1-16-g0c4474d then I want to build a libvirt package with name like libvirt-1.2.14-1.el7.v1.2.14-rc1-16-g0c4474d.x86_64 The context why I need this is there are some hosts running scripts for auto QE-consumption testing while there is no job running sometimes.( we won't get a internal release version everyday, it is not necessary also). We can make use of these resource to do some auto testing daily so that we may be able to find the regression problems earlier. So, a package with commit info is required for comparative testing. After talk with eblake I got a way as below: after 'make rpm' , I will get libvirt.spec, then I can tweak libvirt.spec add a line "%define extra_release .git_commit_num" at the top of libvirt.spec then use the tweaked libvirt.spec to build the package I want to get I wonder anybody have a better method to achieve this ? Thanks -- Regards shyu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Lispvirt: porting Libvirt API for Common Lisp
Hi everyone! I'm developing a Libvirt bindings for Common Lisp. The project is called "Lispvirt". I created this project because I was doing a project in Lisp to manage Virtual Machines. So, I needed to implement some code using C and set up Lisp to access those methods in C. This project was becoming a mess. The better scenario is using only Lisp. That's why I started to develop this bindings for Lisp. Now, I'm only using Lisp for it. For a while, I'm hosting this project on GitHub: https://github.com/jcfaracco/lispvirt But I'm planning to move it to common-lisp.net. There is still many things to do (callbacks, structures, some project decisions and planings, documentation, etc). Any contribution or suggestions would be helpful. The most important things to do now are test, test and test. Just sharing if someone is interested to help us. Thanks! *--* *Julio Cesar Faracco* -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add vmport feature
Hi On Wed, Apr 1, 2015 at 2:45 AM, Marc-André Lureau < marcandre.lur...@gmail.com> wrote: > +Since 1.2.14 I guess this will have to wait 1.2.15 -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add vmport feature
The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm. However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications. --- docs/formatdomain.html.in | 6 + docs/schemas/domaincommon.rng | 9 +++ src/conf/domain_conf.c | 5 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 14 +++ .../qemuxml2argv-machine-vmport-opt.args | 6 + .../qemuxml2argv-machine-vmport-opt.xml| 28 ++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b496c3..faa22ec 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1500,6 +1500,12 @@ performance monitoring unit for the guest. Since 1.2.12 + vmport + Depending on the state attribute (values on, +off, default on) enable or disable the +the emulation of VMWare IO port, for vmmouse etc. +Since 1.2.14 + Time keeping diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 03fd541..b8e06b3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4149,6 +4149,15 @@ + + + + + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cd6ee22..b92c634 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "kvm", "pvspinlock", "capabilities", - "pmu") + "pmu", + "vmport") VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, "default", @@ -14223,6 +14224,7 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: +case VIR_DOMAIN_FEATURE_VMPORT: node = ctxt->node; ctxt->node = nodes[i]; if ((tmp = virXPathString("string(./@state)", ctxt))) { @@ -20858,6 +20860,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: +case VIR_DOMAIN_FEATURE_VMPORT: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 84e880a..b29767e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1634,6 +1634,7 @@ typedef enum { VIR_DOMAIN_FEATURE_PVSPINLOCK, VIR_DOMAIN_FEATURE_CAPABILITIES, VIR_DOMAIN_FEATURE_PMU, +VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce6767c..8782613 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -279,6 +279,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "qxl.vgamem_mb", "qxl-vga.vgamem_mb", "pc-dimm", + + "machine-vmport-opt", /* 185 */ ); @@ -3239,6 +3241,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT); +/* vmport option is supported v2.2.0 onwards */ +if (qemuCaps->version >= 2002000) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); + /* WebSockets were introduced between 1.3.0 and 1.3.1 */ if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c7b1ac7..48c8f96 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src
Re: [libvirt] [PATCH 1/2] tests: nodeinfo: Test F21 aarch64 on APM mustang
On 03/31/2015 04:19 PM, Eric Blake wrote: > On 03/31/2015 09:19 AM, Cole Robinson wrote: >> On 03/31/2015 06:06 AM, Ján Tomko wrote: >>> On Sat, Mar 28, 2015 at 02:31:30PM -0400, Cole Robinson wrote: >>> -- 8< -- >>> >>> ACK to both. >>> >>> Jan >>> >> >> Thanks, but trying to push I'm getting errors: >> >> remote: tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/uevent:1: new blank >> line >> at EOF. > > I'll push on your behalf (I know the magic secrets for temporarily > relaxing the libvirt.org commit checks in cases like this. Supposedly, > it was at one point possible for anyone to make those changes on a > one-push basis by creating a special tag for the commit, but without > actually reading the hooks on libvirt.org, and in light of no > documentation for such a hack, I think that the only people that can do > it right now are those with libvirt.org shell login; it would be nice to > get to a point where anyone with push rights could do it even without > shell rights, but it happens so seldom that no one is ever motivated to > tackle that project) > > >> And now that I actually run syntax-check (sorry), seeing an additional error: >> >> tests/nodeinfodata/linux-f21-mustang/cpu/offline:1: >> tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/offline:1: >> maint.mk: Prohibited empty first line >> cfg.mk:956: recipe for target 'sc_prohibit_empty_first_line' failed >> make: *** [sc_prohibit_empty_first_line] Error 1 > > And the fix for this is: > > diff --git i/cfg.mk w/cfg.mk > index 661..79c5d48 100644 > --- i/cfg.mk > +++ w/cfg.mk > @@ -1185,7 +1185,7 @@ > exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \ > > ^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$ > > exclude_file_name_regexp--sc_prohibit_empty_first_line = \ > - > ^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt|tests/nodeinfodata/linux-raspberrypi/cpu/offline)$$ > + > ^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/(vmware|nodeinfo)data/*)$$ > > exclude_file_name_regexp--sc_prohibit_useless_translation = \ >^tests/virpolkittest.c > > >> >> >> I attached the unaltered patches if anyone wants to figure out the tweaks to >> make these bits pass, I can't look at this at the moment. > > I'll push shortly. > Thanks a lot Eric! - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: nodeinfo: Test F21 aarch64 on APM mustang
On 03/31/2015 09:19 AM, Cole Robinson wrote: > On 03/31/2015 06:06 AM, Ján Tomko wrote: >> On Sat, Mar 28, 2015 at 02:31:30PM -0400, Cole Robinson wrote: >> -- 8< -- >> >> ACK to both. >> >> Jan >> > > Thanks, but trying to push I'm getting errors: > > remote: tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/uevent:1: new blank line > at EOF. I'll push on your behalf (I know the magic secrets for temporarily relaxing the libvirt.org commit checks in cases like this. Supposedly, it was at one point possible for anyone to make those changes on a one-push basis by creating a special tag for the commit, but without actually reading the hooks on libvirt.org, and in light of no documentation for such a hack, I think that the only people that can do it right now are those with libvirt.org shell login; it would be nice to get to a point where anyone with push rights could do it even without shell rights, but it happens so seldom that no one is ever motivated to tackle that project) > And now that I actually run syntax-check (sorry), seeing an additional error: > > tests/nodeinfodata/linux-f21-mustang/cpu/offline:1: > tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/offline:1: > maint.mk: Prohibited empty first line > cfg.mk:956: recipe for target 'sc_prohibit_empty_first_line' failed > make: *** [sc_prohibit_empty_first_line] Error 1 And the fix for this is: diff --git i/cfg.mk w/cfg.mk index 661..79c5d48 100644 --- i/cfg.mk +++ w/cfg.mk @@ -1185,7 +1185,7 @@ exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \ ^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$ exclude_file_name_regexp--sc_prohibit_empty_first_line = \ - ^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt|tests/nodeinfodata/linux-raspberrypi/cpu/offline)$$ + ^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/(vmware|nodeinfo)data/*)$$ exclude_file_name_regexp--sc_prohibit_useless_translation = \ ^tests/virpolkittest.c > > > I attached the unaltered patches if anyone wants to figure out the tweaks to > make these bits pass, I can't look at this at the moment. I'll push shortly. -- 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] libxl: fix dom0 balloon logic
On Tue, Mar 31, 2015 at 08:16:09AM -0600, Jim Fehlig wrote: Martin Kletzander wrote: On Tue, Mar 24, 2015 at 02:43:42PM -0600, Jim Fehlig wrote: Recent testing on large memory systems revealed a bug in the Xen xl tool's freemem() function. When autoballooning is enabled, freemem() is used to ensure enough memory is available to start a domain, ballooning dom0 if necessary. When ballooning large amounts of memory from dom0, freemem() would exceed its self-imposed wait time and return an error. Meanwhile, dom0 continued to balloon. Starting the domain later, after sufficient memory was ballooned from dom0, would succeed. The libvirt implementation in libxlDomainFreeMem() suffers the same bug since it is modeled after freemem(). In the end, the best place to fix the bug on the Xen side was to slightly change the behavior of libxl_wait_for_memory_target(). Instead of failing after caller-provided wait_sec, the function now blocks as long as dom0 memory ballooning is progressing. It will return failure only when more memory is needed to reach the target and wait_sec have expired with no progress being made. See xen.git commit fd3aa246. There was a dicussion on how this would affect other libxl apps like libvirt http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html If libvirt containing this patch was build against a Xen containing the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() will fail after 30 sec and domain creation will be terminated. Without this patch and with old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() does not succeed after 30 sec, but returns success anyway. Domain creation continues resulting in all sorts of fun stuff like cpu soft lockups in the guest OS. It was decided to properly fix libxl_wait_for_memory_target(), and if anything improve the default behavior of apps using the freemem reference impl in xl. xl was patched to accommodate the change in libxl_wait_for_memory_target() with xen.git commit 883b30a0. This patch does the same in the libxl driver. While at it, I changed the logic to essentially match freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner IMO and will make it easier to spot future, potentially interesting divergences. Signed-off-by: Jim Fehlig --- V2: Actually use libxl_wait_for_memory_target(), instead of libxl_wait_for_free_memory() src/libxl/libxl_domain.c | 55 +++- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 407a9bd..a1739aa 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1121,38 +1121,39 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) { uint32_t needed_mem; uint32_t free_mem; -size_t i; -int ret = -1; +int ret; This variable is unnecessary and confusing since you don't return it. And without this, you can make ... int tries = 3; int wait_secs = 10; -if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, -&needed_mem)) >= 0) { -for (i = 0; i < tries; ++i) { -if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) < 0) -break; +ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, &needed_mem); +if (ret < 0) +goto error; ... these conditions smaller, e.g.: if (libxl_domain_need_memory(priv->ctx, &d_config->b_info, &needed_mem) < 0) goto error; Yeah, too much copy and paste. I'll fix this up and send a V3. Do you need this for the release? (sorry for noticing that late) No, this can wait. From a Xen perspective, more useful work for the release would have been this series https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html But I think it is a bit too late now :-(. I missed that, sorry. I'll have a look at it tomorrow. Not being that Xen-knowledgeable, I probably haven't felt that confident reviewing that. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgp_34bWKZnxE.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] libxl: fix dom0 balloon logic
Martin Kletzander wrote: > On Tue, Mar 24, 2015 at 02:43:42PM -0600, Jim Fehlig wrote: >> Recent testing on large memory systems revealed a bug in the Xen xl >> tool's freemem() function. When autoballooning is enabled, freemem() >> is used to ensure enough memory is available to start a domain, >> ballooning dom0 if necessary. When ballooning large amounts of memory >> from dom0, freemem() would exceed its self-imposed wait time and >> return an error. Meanwhile, dom0 continued to balloon. Starting the >> domain later, after sufficient memory was ballooned from dom0, would >> succeed. The libvirt implementation in libxlDomainFreeMem() suffers >> the same bug since it is modeled after freemem(). >> >> In the end, the best place to fix the bug on the Xen side was to >> slightly change the behavior of libxl_wait_for_memory_target(). >> Instead of failing after caller-provided wait_sec, the function now >> blocks as long as dom0 memory ballooning is progressing. It will return >> failure only when more memory is needed to reach the target and wait_sec >> have expired with no progress being made. See xen.git commit fd3aa246. >> There was a dicussion on how this would affect other libxl apps like >> libvirt >> >> http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html >> >> If libvirt containing this patch was build against a Xen containing >> the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() >> will fail after 30 sec and domain creation will be terminated. >> Without this patch and with old libxl_wait_for_memory_target() behavior, >> libxlDomainFreeMem() does not succeed after 30 sec, but returns success >> anyway. Domain creation continues resulting in all sorts of fun stuff >> like cpu soft lockups in the guest OS. It was decided to properly fix >> libxl_wait_for_memory_target(), and if anything improve the default >> behavior of apps using the freemem reference impl in xl. >> >> xl was patched to accommodate the change in >> libxl_wait_for_memory_target() >> with xen.git commit 883b30a0. This patch does the same in the libxl >> driver. While at it, I changed the logic to essentially match >> freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner >> IMO and will make it easier to spot future, potentially interesting >> divergences. >> >> Signed-off-by: Jim Fehlig >> --- >> >> V2: Actually use libxl_wait_for_memory_target(), instead of >>libxl_wait_for_free_memory() >> >> src/libxl/libxl_domain.c | 55 >> +++- >> 1 file changed, 26 insertions(+), 29 deletions(-) >> >> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c >> index 407a9bd..a1739aa 100644 >> --- a/src/libxl/libxl_domain.c >> +++ b/src/libxl/libxl_domain.c >> @@ -1121,38 +1121,39 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr >> priv, libxl_domain_config *d_config) >> { >> uint32_t needed_mem; >> uint32_t free_mem; >> -size_t i; >> -int ret = -1; >> +int ret; > > This variable is unnecessary and confusing since you don't return it. > And without this, you can make ... > >> int tries = 3; >> int wait_secs = 10; >> >> -if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, >> -&needed_mem)) >= 0) { >> -for (i = 0; i < tries; ++i) { >> -if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) >> < 0) >> -break; >> +ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, >> &needed_mem); >> +if (ret < 0) >> +goto error; >> > > ... these conditions smaller, e.g.: > > if (libxl_domain_need_memory(priv->ctx, &d_config->b_info, > &needed_mem) < 0) > goto error; Yeah, too much copy and paste. I'll fix this up and send a V3. > Do you need this for the release? (sorry for noticing that late) No, this can wait. From a Xen perspective, more useful work for the release would have been this series https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html But I think it is a bit too late now :-(. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Fix cgroups regresion when default cpuset is specified
On Fri, Mar 27, 2015 at 02:12:04PM +0100, Peter Krempa wrote: > Since commit a39f69d2b libvirt would fail to start a VM if the default cpu set > was specified and individual vcpus were pinned to cpus outside of that cpuset. > > Peter Krempa (8): > qemu: cgroup: Store auto cpuset instead of re-creating it on demand > qemu: cgroup: Refactor setup for IOThread cgroups > qemu: cgroup: Properly set up vcpu pinning > qemu: cgroup: Use priv->autoCpuset instead of using > qemuPrepareCpumap() > qemu: cgroup: Rename qemuSetupCgroupEmulatorPin to > qemuSetupCgroupCpusetCpus > qemu: cgroup: Kill qemuSetupCgroupIOThreadsPin() > qemu: cgroup: Kill qemuSetupCgroupVcpuPin() > qemu: Copy bitmap in a sane way > > src/qemu/qemu_cgroup.c | 150 > +++- > src/qemu/qemu_cgroup.h | 13 + > src/qemu/qemu_domain.c | 1 + > src/qemu/qemu_domain.h | 3 + > src/qemu/qemu_driver.c | 21 +++ > src/qemu/qemu_process.c | 93 +- > src/qemu/qemu_process.h | 2 - > 7 files changed, 85 insertions(+), 198 deletions(-) > ACK series. Nice cleanup, that will show the NUMA topology witch! [1] Jan [1] https://www.redhat.com/archives/libvir-list/2015-March/msg01581.html signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: add check for exclusive options for domstats command
On Tue, Mar 31, 2015 at 15:34:10 +0200, Pavel Hrdina wrote: > Option --domain cannot be combined with options --list-*. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143837 > > Signed-off-by: Pavel Hrdina > --- > tools/virsh-domain-monitor.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c > index 6951db2..e683a6d 100644 > --- a/tools/virsh-domain-monitor.c > +++ b/tools/virsh-domain-monitor.c > @@ -2110,6 +2110,15 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) > const vshCmdOpt *opt = NULL; > bool ret = false; > > +VSH_EXCLUSIVE_OPTIONS("domain", "list-active"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-inactive"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-persistent"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-transient"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-running"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-paused"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-shutoff"); > +VSH_EXCLUSIVE_OPTIONS("domain", "list-other"); > + > if (vshCommandOptBool(cmd, "state")) > stats |= VIR_DOMAIN_STATS_STATE; NACK, it was a deliberate design decision to allow users provide filters along with domain lists with the intention that we might want to filter the list provided by users. It was never implemented though. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: add check for exclusive options for domstats command
Option --domain cannot be combined with options --list-*. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143837 Signed-off-by: Pavel Hrdina --- tools/virsh-domain-monitor.c | 9 + 1 file changed, 9 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 6951db2..e683a6d 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2110,6 +2110,15 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) const vshCmdOpt *opt = NULL; bool ret = false; +VSH_EXCLUSIVE_OPTIONS("domain", "list-active"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-inactive"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-persistent"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-transient"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-running"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-paused"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-shutoff"); +VSH_EXCLUSIVE_OPTIONS("domain", "list-other"); + if (vshCommandOptBool(cmd, "state")) stats |= VIR_DOMAIN_STATS_STATE; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/9] qemu: Add quorum support to libvirt
On Tue, Mar 17, 2015 at 8:25 PM, Matthias Gatto wrote: > The purpose of these patches is to introduce quorum for libvirt > I've try to follow this proposal: > http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html > > This feature ask for 6 task: > > 1) Allow a _virStorageSource to contain more than one backing store. > > Because all the actual libvirt code use the backingStore field > as a pointer and we needs want to change that, I've decide to encapsulate > the backingStore field to simplifie the array manipulation. > > 2) Add the missing field a quorum need in _virStorageSource and > the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in > their respectives enums. > > 3) Parse and format the xml > Because a quorum allows to have more than one backing store at the same level > we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML > to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse > in a loop. > virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can > call themself recursively in a loop because a quorum can contain another > quorum > > 4) Add nodename > We need to add nodename support in _virStorageSource because qemu > use them for their child. > > 5) Build qemu string > As for the xml, we have to call the function which create quorum recursively. > But this task have the problem explained here: > http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html > The _virStorageSource missing some informations that can be passed to > a child, and therefore this version of quorum is incomplet. > > 6) Allow to hotplug/change a disk in a quorum > This part is not present in these patches because for this task > we have to use blockdev-add, and currently libvirt use > device_add for hotpluging that doesn't allow to hotplug quorum childs. > > There is 3 way to handle this problem: > 1) create a virDomainBlockDevAdd function in libvirt witch call > blockdev-add. > > 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice > > 3) write a hack which uses blockdev-add when only attaching quorum > (but i'm pretty sure this solution is not the good one) > > V2: > -Rebase on master > -Add Documentation > > V3: > -Transforme the backingStore field in virStorageSource into > an array of pointer instead of a pointer > -Modify virStorageSourceSetBackingStore to allow it to expand > the backingStore size. > > V4: > -Rebase on master > > Matthias Gatto (9): > virstoragefile: Add virStorageSourceGetBackingStore > virstoragefile: Always use virStorageSourceGetBackingStore to get > backing store > virstoragefile: Add virStorageSourceSetBackingStore > virstoragefile: Always use virStorageSourceSetBackingStore to set > backing store > virstoragefile: change backingStore to backingStores. > virstoragefile: Add quorum in virstoragefile > domain_conf: Read and Write quorum config > qemu: Add quorum support in qemuBuildDriveDevStr > virstoragefile: Add node-name > > docs/formatdomain.html.in | 27 - > docs/schemas/domaincommon.rng | 95 +++-- > docs/schemas/storagecommon.rng| 1 + > docs/schemas/storagevol.rng | 1 + > src/conf/domain_conf.c| 195 > ++ > src/conf/storage_conf.c | 22 ++-- > src/libvirt_private.syms | 2 + > src/qemu/qemu_cgroup.c| 4 +- > src/qemu/qemu_command.c | 114 > src/qemu/qemu_domain.c| 2 +- > src/qemu/qemu_driver.c| 30 +++--- > src/qemu/qemu_migration.c | 1 + > src/security/security_dac.c | 2 +- > src/security/security_selinux.c | 4 +- > src/security/virt-aa-helper.c | 2 +- > src/storage/storage_backend.c | 35 +++--- > src/storage/storage_backend_fs.c | 37 --- > src/storage/storage_backend_gluster.c | 10 +- > src/storage/storage_backend_logical.c | 15 ++- > src/storage/storage_driver.c | 2 +- > src/util/virstoragefile.c | 116 +--- > src/util/virstoragefile.h | 13 ++- > tests/virstoragetest.c| 18 ++-- > 23 files changed, 573 insertions(+), 175 deletions(-) > > -- > 1.8.3.1 > ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virDomainShutdown*: Describe blocking behaviour
On 03/31/2015 01:37 AM, Michal Privoznik wrote: >> I don't think this is the right approach. It is okay to block for a >> small finite amount of time, but if the guest agent really is something >> that can hang, we should fix that code to return without waiting for the >> agent response. >> > > The problem is, we can tell if the shutdown qemu-ga command succeeded > only when we see the SHUTDOWN event on the domain monitor. But we don't NEED to know if it succeeded - per our docs, we only issue the request (or fail outright because the request cannot be delivered because the agent is non-responsive). > The other > option is to treat it like we're treating ACPI mode of the API: just > send the request and report if the sending succeeded. Don't wait until > domain actually shuts down. I can provide a patch. Yes, that would be more in line with the docs. -- 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] connect: ssh: Shall we remove the dependency of netcat ?
On Tue, Mar 31, 2015 at 09:02:23 +0800, zhang bo wrote: > On 2015/3/28 0:25, Peter Krempa wrote: > > > On Fri, Mar 27, 2015 at 10:54:26 +0800, zhang bo wrote: > > > > > Too powerful? That is a ridiculous statement that probably originates > > from some kind of misunderstanding when creating a security policy or > > stuff like that. If a policy bans nc as "powerful" then it's missing on > > a lot of other options how to create listening or outgoing connections > > on arbitrary sockets. The only insecure part is that it does not use > > encryption, but that's a widely known fact about nc. > > > > > Sorry for replying so late:) > I tried to confirm the security fact the other days, unfortunately no clear > answer gotten. > What I meant was that the *network monitoring tools*, such as 'nc' and > 'tcpdump', > they are considered as dangerous for network security. They are prohibited > in our company and some other organizations. I'm not quite sure whether the > result that > they're prohibited are directly related to their capability of monitoring > network. > But they actually got prohibited anyway. That sounds like a security-by-obscurity policy. I don't think that banning such tools might have any benefit for security. Anyways I'm planing on adding the native client. In such case, companies having such ridiculous security rules may opt to uninstall netcat and rely solely on libvirt's internal client (once it's implemented). Such policy will then basically mandate a minimal version of libvirt that will support the new client as older clients will still want to use NC. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: blockjob: Synchronously update backing chain in XML on ABORT/PIVOT
On Mon, Mar 30, 2015 at 14:51:08 -0600, Eric Blake wrote: > On 03/30/2015 03:26 AM, Peter Krempa wrote: > > When the synchronous pivot option is selected, libvirt would not update > > the backing chain until the job was exitted. Some applications then > > s/exitted/exited/ > > > received invalid data as their job serialized first. > > > > This patch removes polling to wait for the ABORT/PIVOT job completion > > and replaces it with a condition. If a synchronous operation is > > requested the update of the XML is executed in the job of the caller of > > the synchronous request. Otherwise the monitor event callback uses a > > separate worker to update the backing chain with a new job. > > > > This is a regression since 1a92c719101e5bfa6fe2b78006ad04c7f075ea28 > > unreleased, and therefore worth fixing during freeze. > > > > > When the ABORT job is finished synchronously you get the following call > > stack: > > #0 qemuBlockJobEventProcess > > #1 qemuDomainBlockJobImpl > > #2 qemuDomainBlockJobAbort > > #3 virDomainBlockJobAbort > > > > While previously or while using the _ASYNC flag you'd get: > > #0 qemuBlockJobEventProcess > > #1 processBlockJobEvent > > #2 qemuProcessEventHandler > > #3 virThreadPoolWorker > > --- > > src/conf/domain_conf.c | 16 +++- > > src/conf/domain_conf.h | 6 ++ > > src/qemu/qemu_driver.c | 45 +++-- > > src/qemu/qemu_process.c | 38 +- > > 4 files changed, 65 insertions(+), 40 deletions(-) > > > > > @@ -16389,37 +16395,24 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, > > status); > > event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, > > status); > > -} else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) { > > +} else if (disk->blockJobSync) { > > /* XXX If the event reports failure, we should reflect > > * that back into the return status of this API call. */ > > Isn't this comment stale now? That is, since you are waiting for the > event to free the condition, and we are listening for both success and > failure events, we can now tell if the event reports failure. No it is not. The code currently does not change the return value depending on the success state of the operation. I wanted not to change semantics in this case. I'm going to follow up to this series with a series that refactors qemuDomainBlockJobImpl by splitting it into seprate functions instead of that spaghetti monster of function it is now. In that change I'll then try to fix the semantics. > > Otherwise, looks correct to me. I hate that it is so close to the > release deadline, but I hate regressions more (which is why we have > freezes). I'd feel better if you got a second review, if you can > wrangle one up in time, but here's my: > > ACK. > Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] iscsi: Check for presence of error before creating new one
On 03/31/2015 07:03 AM, Ján Tomko wrote: > On Mon, Mar 30, 2015 at 07:16:32PM -0400, John Ferlan wrote: >> If virStorageBackendSCSIFindLUs fails, but the failure has an error >> message - the iscsi code didn't honor that creating it's own wonderful >> message such as "error: Failed to find LUs on host 60: ..." - not overly >> helpful. Since a few of the called paths generate a message, check for >> that before using that generic one. > > Both of the paths returning -1 generate an error: > * return -1 after opendir failed > * retval = virDirRead > >> >> Signed-off-by: John Ferlan >> --- >> src/storage/storage_backend_iscsi.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/src/storage/storage_backend_iscsi.c >> b/src/storage/storage_backend_iscsi.c >> index 079c767..1dac238 100644 >> --- a/src/storage/storage_backend_iscsi.c >> +++ b/src/storage/storage_backend_iscsi.c >> @@ -147,8 +147,9 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, >> } >> >> if (virStorageBackendSCSIFindLUs(pool, host) < 0) { >> -virReportSystemError(errno, >> - _("Failed to find LUs on host %u"), host); > >> +if (virGetLastError() == NULL) > > This condition is never true. And it should only be used if > we can't fix the function to consistently set an error in all > cases. > Fair enough - just being overly cautious or un-optimistic I suppose John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] qemuProcessHook: Call virNuma*() iff needed
https://bugzilla.redhat.com/show_bug.cgi?id=1198645 Once upon a time, there was a little domain. And the domain was pinned onto a NUMA node and hasn't fully allocated its memory: 2355200 1048576 Oh little me, said the domain, what will I do with so little memory. If I only had a few megabytes more. But the old admin noticed the whimpering, barely audible to untrained human ear. And good admin he was, he gave the domain yet more memory. But the old NUMA topology witch forbade to allocate more memory on the node zero. So he decided to allocate it on a different node: virsh # numatune little_domain --nodeset 0-1 virsh # setmem little_domain 2355200 The little domain was happy. For a while. Until bad, sharp teeth shaped creature came. Every process in the system was afraid of him. The OOM Killer they called him. Oh no, he's after the little domain. There's no escape. Do you kids know why? Because when the little domain was born, her father, Libvirt, called numa_set_membind(). So even if the admin allowed her to allocate memory from other nodes in the cgroups, the membind() forbid it. So what's the lesson? Libvirt should rely on cgroups, whenever possible and use numa_set_membind() as the last ditch effort. Signed-off-by: Michal Privoznik --- src/qemu/qemu_process.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2d86eb8..6c955c3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3166,6 +3166,7 @@ static int qemuProcessHook(void *data) int fd; virBitmapPtr nodeset = NULL; virDomainNumatuneMemMode mode; +bool doNuma = true; /* This method cannot use any mutexes, which are not * protected across fork() @@ -3197,11 +3198,23 @@ static int qemuProcessHook(void *data) goto cleanup; mode = virDomainNumatuneGetMode(h->vm->def->numa, -1); -nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa, - priv->autoNodeset, -1); -if (virNumaSetupMemoryPolicy(mode, nodeset) < 0) -goto cleanup; +if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && +h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) && +virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) { +/* Use virNuma* API iff necessary. Once set and child is exec()-ed, + * there's no way for us to change it. Rely on cgroups (if available + * and enabled in the config) rather then virNuma*. */ +doNuma = false; +} + +if (doNuma) { +nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa, + priv->autoNodeset, -1); + +if (virNumaSetupMemoryPolicy(mode, nodeset) < 0) +goto cleanup; +} ret = 0; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] A few NUMA fixes
diff to v1: -reworked to follow Jan's review. Hopefully. Michal Privoznik (3): vircgroup: Introduce virCgroupControllerAvailable qemuProcessHook: Call virNuma*() iff needed virLXCControllerSetupResourceLimits: Call virNuma*() iff needed src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 22 -- src/qemu/qemu_process.c | 21 + src/util/vircgroup.c | 19 +++ src/util/vircgroup.h | 1 + tests/vircgrouptest.c| 31 +++ 6 files changed, 85 insertions(+), 10 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] virLXCControllerSetupResourceLimits: Call virNuma*() iff needed
Like we are doing in qemu driver ($COMMIT_HASH_TO_BE_FILLED_DURING_PUSHING), lets call virNumaSetupMemoryPolicy() only if really needed. Problem is, if we numa_set_membind() child, there's no way to change it from the daemon afterwards. So any later attempts to change the pinning will fail. But in very weird way - CGroups will be set, but due to membind child will not allocate memory from any other node. Signed-off-by: Michal Privoznik --- src/lxc/lxc_controller.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8545f29..4b340ab 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -742,14 +742,24 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl) virBitmapPtr nodeset = NULL; virDomainNumatuneMemMode mode; -VIR_DEBUG("Setting up process resource limits"); - -if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0) -goto cleanup; - -nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1); mode = virDomainNumatuneGetMode(ctrl->def->numa, -1); +if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && +virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) { +/* Use virNuma* API iff necessary. Once set and child is exec()-ed, + * there's no way for us to change it. Rely on cgroups (if available + * and enabled in the config) rather then virNuma*. */ +VIR_DEBUG("Postponing setting up resource limits to CGroup set up phase"); +return virLXCControllerSetupCpuAffinity(ctrl); +} + +VIR_DEBUG("Setting up process resource limits"); + +if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0) +goto cleanup; + +nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1); + if (virNumaSetupMemoryPolicy(mode, nodeset) < 0) goto cleanup; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] vircgroup: Introduce virCgroupControllerAvailable
This new internal API checks if given CGroup controller is available. It is going to be needed later when we need to make a decision whether pin domain memory onto NUMA nodes using cpuset CGroup controller or using numa_set_membind(). Signed-off-by: Michal Privoznik --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 19 +++ src/util/vircgroup.h | 1 + tests/vircgrouptest.c| 31 +++ 4 files changed, 52 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5716ece..136036b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1129,6 +1129,7 @@ virCgroupAllowDevice; virCgroupAllowDeviceMajor; virCgroupAllowDevicePath; virCgroupAvailable; +virCgroupControllerAvailable; virCgroupControllerTypeFromString; virCgroupControllerTypeToString; virCgroupDenyAllDevices; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d42f433..00c0bab 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -4011,6 +4011,20 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller) return ret; } +bool +virCgroupControllerAvailable(int controller) +{ +virCgroupPtr cgroup; +bool ret = false; + +if (virCgroupNewSelf(&cgroup) < 0) +return ret; + +ret = virCgroupHasController(cgroup, controller); +virCgroupFree(&cgroup); +return ret; +} + #else /* !VIR_CGROUP_SUPPORTED */ bool @@ -4781,4 +4795,9 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup ATTRIBUTE_UNUSED, return -1; } +bool +virCgroupControllerAvailable(int controller ATTRIBUTE_UNUSED) +{ +return false; +} #endif /* !VIR_CGROUP_SUPPORTED */ diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index eee15ca..5d6356d 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -282,4 +282,5 @@ int virCgroupSetOwner(virCgroupPtr cgroup, int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller); +bool virCgroupControllerAvailable(int controller); #endif /* __VIR_CGROUP_H__ */ diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index bbf28d1..2b30258 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -586,6 +586,34 @@ static int testCgroupAvailable(const void *args) return 0; } +static int testCgroupControllerAvailable(const void *args ATTRIBUTE_UNUSED) +{ +int ret = 0; + +# define CHECK_CONTROLLER(c, present) \ +if ((present && !virCgroupControllerAvailable(c)) ||\ +(!present && virCgroupControllerAvailable(c))) {\ +fprintf(stderr, present ? \ +"Expected controller %s not available\n" : \ +"Unexpected controller %s available\n", #c);\ +ret = -1; \ +} + +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_CPU, true) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_CPUACCT, true) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_CPUSET, true) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_MEMORY, true) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_DEVICES, false) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_FREEZER, true) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_BLKIO, true) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_NET_CLS, false) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_PERF_EVENT, false) +CHECK_CONTROLLER(VIR_CGROUP_CONTROLLER_SYSTEMD, true) + +# undef CHECK_CONTROLLER +return ret; +} + static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) { virCgroupPtr cgroup = NULL; @@ -886,6 +914,9 @@ mymain(void) if (virtTestRun("Cgroup available", testCgroupAvailable, (void*)0x1) < 0) ret = -1; +if (virtTestRun("Cgroup controller available", testCgroupControllerAvailable, NULL) < 0) +ret = -1; + if (virtTestRun("virCgroupGetBlkioIoServiced works", testCgroupGetBlkioIoServiced, NULL) < 0) ret = -1; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] scsi: Check for invalid target.path after processLU failure
On Mon, Mar 30, 2015 at 07:16:34PM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1171933 > > After processing all the LU's and find no "real" LU's - check if perhaps > the cause of that was a poorly formed 'target.path'. The expection is > generally that the path is /dev/disk/by-path or at least something in /dev. > Although it's not impossible for the user to provide something. If they > do provide something it should be valid or we should just cause a failure > to start the pool with an appropriate message. > > Signed-off-by: John Ferlan > --- > src/storage/storage_backend_scsi.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/src/storage/storage_backend_scsi.c > b/src/storage/storage_backend_scsi.c > index 2f1f5ed..c3a1892 100644 > --- a/src/storage/storage_backend_scsi.c > +++ b/src/storage/storage_backend_scsi.c > @@ -467,6 +467,15 @@ > virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr pool, > if (!*found) > VIR_DEBUG("No LU found for pool %s", pool->def->name); > > +if (!*found && !virFileExists(pool->def->target.path) && > +!STRPREFIX(pool->def->target.path, "/dev")) { Checking for *found here seems pointless. After the logic change in the previous patch, it is implied by either of the following conditions: a) If the target path does not start with "/dev", *found will be false: virStorageBackendStablePath will short-circuit, just strduping the volume path, and virStorageBackendSCSINewLun will return -1 in that case. b) If the target path does not exist, it will either be rejected by the above code path, or by the failed opendir in virStorageBackendStablePath. If all you want to do is forbid to start an {,i}SCSI pool that does not start with /dev, we can do that much earlier in {,I}SCSIStartPool. To catch wrong paths in /dev, I think the proper way is to stop ignoring the return value of processLU and make it return -1 on fatal errors (OOM, pool target path does not exist, etc.) and -2 on errors that won't necessarily stop us from finding other volumes. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] iscsi: Check for presence of error before creating new one
On Mon, Mar 30, 2015 at 07:16:32PM -0400, John Ferlan wrote: > If virStorageBackendSCSIFindLUs fails, but the failure has an error > message - the iscsi code didn't honor that creating it's own wonderful > message such as "error: Failed to find LUs on host 60: ..." - not overly > helpful. Since a few of the called paths generate a message, check for > that before using that generic one. Both of the paths returning -1 generate an error: * return -1 after opendir failed * retval = virDirRead > > Signed-off-by: John Ferlan > --- > src/storage/storage_backend_iscsi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/storage/storage_backend_iscsi.c > b/src/storage/storage_backend_iscsi.c > index 079c767..1dac238 100644 > --- a/src/storage/storage_backend_iscsi.c > +++ b/src/storage/storage_backend_iscsi.c > @@ -147,8 +147,9 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, > } > > if (virStorageBackendSCSIFindLUs(pool, host) < 0) { > -virReportSystemError(errno, > - _("Failed to find LUs on host %u"), host); > +if (virGetLastError() == NULL) This condition is never true. And it should only be used if we can't fix the function to consistently set an error in all cases. Jan > +virReportSystemError(errno, > + _("Failed to find LUs on host %u"), host); > retval = -1; > } signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: nodeinfo: Test F21 aarch64 on APM mustang
On Sat, Mar 28, 2015 at 02:31:30PM -0400, Cole Robinson wrote: -- 8< -- ACK to both. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] hostdev: fix net config restore error
On Thu, Mar 26, 2015 at 10:23:47PM +0800, Huanle Han wrote: Fix for such a case: 1. Domain A and B xml contain the same SRIOV net hostdev( with same pci address). 2. virsh start A (Successfully, and configure the SRIOV net with custom mac) 3. virsh start B (Fail because of the hostdev used by domain A or other reason.) In step 3, 'virHostdevNetConfigRestore' is called for the hostdev which is still used by domain A. It makes the mac/vlan of the SRIOV net change. Makes perfect sense, thanks for finding that out. Code Change in this fix: 1. As the pci used by other domain have been removed from 'pcidevs' in previous loop, we only restore the nic config for the hostdev still in 'pcidevs'(used by this domain) 2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the hostdev is a pci net device or not. 3. update the comments to make it more clear Signed-off-by: Huanle Han --- src/util/virhostdev.c | 52 --- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 83f567d..b04bae2 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -350,6 +350,14 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, return ret; } +static int +virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) +{ +return hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && +hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && +hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && +hostdev->parent.data.net; +} static int virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, @@ -481,10 +489,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, /* This is only needed for PCI devices that have been defined * using . For all others, it is a NOP. */ -if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || -hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || -hostdev->parent.type != VIR_DOMAIN_DEVICE_NET || -!hostdev->parent.data.net) +if (!virHostdevIsPCINetDevice(hostdev)) return 0; isvf = virHostdevIsVirtualFunction(hostdev); @@ -781,19 +786,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, goto cleanup; } -/* Again 4 loops; mark all devices as inactive before reset +/* Here are 4 loops; mark all devices as inactive before reset * them and reset all the devices before re-attach. * Attach mac and port profile parameters to devices */ + +/* Loop 1: delete the copy of the dev from pcidevs if it's used by + * other domain. Or delete it from activePCIHostDevs if it had + * been used by this domain. + */ i = 0; while (i < virPCIDeviceListCount(pcidevs)) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); virPCIDevicePtr activeDev = NULL; -/* delete the copy of the dev from pcidevs if it's used by - * other domain. Or delete it from activePCIHostDevs if it had - * been used by this domain. - */ activeDev = virPCIDeviceListFind(hostdev_mgr->activePCIHostdevs, dev); if (activeDev) { const char *usedby_drvname; @@ -814,14 +820,27 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, * pcidevs, but has been removed from activePCIHostdevs. */ -/* - * For SRIOV net host devices, unset mac and port profile before - * reset and reattach device +/* Loop 2: before reset and reattach device, + * unset mac and port profile for SRIOV net host devices used by this domain */ The comments were formatted as a sentence, it would be nice to keep it that way. -for (i = 0; i < nhostdevs; i++) -virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr->stateDir, - oldStateDir); +for (i = 0; i < nhostdevs; i++) { +virDomainHostdevDefPtr hostdev = hostdevs[i]; +if (virHostdevIsPCINetDevice(hostdev)) { +virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; +virPCIDevicePtr dev = NULL; +dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); + The daemon will crash if this function returns NULL. There should be check for that, but it shouldn't be fatal, so we can clean up as much as possible (see Loop 3 for example on how to handle that). +if (virPCIDeviceListFind(pcidevs, dev)) { +virHostdevNetConfigRestore(hostdev, hostdev_mgr->stateDir, + oldStateDir); +} +virPCIDeviceFree(dev); +} +} + +/* Loop 3: reset pci device used by this domain */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); @@ -83
Re: [libvirt] [PATCH 1/2] hostdev: Fix index error in loop after remove an element
On Thu, Mar 26, 2015 at 10:23:46PM +0800, Huanle Han wrote: 'virPCIDeviceList' is actually an array. Removing one element makes the rest of the element move. Use while loop, increase index only when not virPCIDeviceListDel(pcidevs, dev) Signed-off-by: Huanle Han --- src/util/virhostdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 23365a3..83f567d 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -785,7 +785,8 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, * them and reset all the devices before re-attach. * Attach mac and port profile parameters to devices */ -for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { +i = 0; +while (i < virPCIDeviceListCount(pcidevs)) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); virPCIDevicePtr activeDev = NULL; @@ -806,6 +807,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr, } virPCIDeviceListDel(hostdev_mgr->activePCIHostdevs, dev); +i++; 'i--;' before that 'continue;' that's not in the index would be shorter, but I guess this is more readable. Or maybe: for (i = virPCIDeviceListCount(pcidevs); i > 0; i--) { size_t dev_index = i - 1; would just deal with this. Anyway, this works just as it is, ACK if you don't want to change it. } /* At this point, any device that had been used by the guest is in -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgpCAeqrCTBlJ.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] libxl: fix dom0 balloon logic
On Tue, Mar 24, 2015 at 02:43:42PM -0600, Jim Fehlig wrote: Recent testing on large memory systems revealed a bug in the Xen xl tool's freemem() function. When autoballooning is enabled, freemem() is used to ensure enough memory is available to start a domain, ballooning dom0 if necessary. When ballooning large amounts of memory from dom0, freemem() would exceed its self-imposed wait time and return an error. Meanwhile, dom0 continued to balloon. Starting the domain later, after sufficient memory was ballooned from dom0, would succeed. The libvirt implementation in libxlDomainFreeMem() suffers the same bug since it is modeled after freemem(). In the end, the best place to fix the bug on the Xen side was to slightly change the behavior of libxl_wait_for_memory_target(). Instead of failing after caller-provided wait_sec, the function now blocks as long as dom0 memory ballooning is progressing. It will return failure only when more memory is needed to reach the target and wait_sec have expired with no progress being made. See xen.git commit fd3aa246. There was a dicussion on how this would affect other libxl apps like libvirt http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html If libvirt containing this patch was build against a Xen containing the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() will fail after 30 sec and domain creation will be terminated. Without this patch and with old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() does not succeed after 30 sec, but returns success anyway. Domain creation continues resulting in all sorts of fun stuff like cpu soft lockups in the guest OS. It was decided to properly fix libxl_wait_for_memory_target(), and if anything improve the default behavior of apps using the freemem reference impl in xl. xl was patched to accommodate the change in libxl_wait_for_memory_target() with xen.git commit 883b30a0. This patch does the same in the libxl driver. While at it, I changed the logic to essentially match freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner IMO and will make it easier to spot future, potentially interesting divergences. Signed-off-by: Jim Fehlig --- V2: Actually use libxl_wait_for_memory_target(), instead of libxl_wait_for_free_memory() src/libxl/libxl_domain.c | 55 +++- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 407a9bd..a1739aa 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1121,38 +1121,39 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) { uint32_t needed_mem; uint32_t free_mem; -size_t i; -int ret = -1; +int ret; This variable is unnecessary and confusing since you don't return it. And without this, you can make ... int tries = 3; int wait_secs = 10; -if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, -&needed_mem)) >= 0) { -for (i = 0; i < tries; ++i) { -if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) < 0) -break; +ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, &needed_mem); +if (ret < 0) +goto error; ... these conditions smaller, e.g.: if (libxl_domain_need_memory(priv->ctx, &d_config->b_info, &needed_mem) < 0) goto error; Do you need this for the release? (sorry for noticing that late) pgpAQl_U8Jp3K.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virDomainShutdown*: Describe blocking behaviour
On 30.03.2015 22:38, Eric Blake wrote: > On 03/30/2015 06:07 AM, Michal Privoznik wrote: >> The description to both virDomainShutdown() and >> virDomainShutdownFlags() is quite misleading when it comes to >> blocking behaviour of these two APIs. Firstly, we support many >> shutdown methods, from signalizing an ACPI event, through sending >> a signal to guest agent assisted shutdown. Some of these methods >> make the API return immediately, while others block the API until >> domain is actually shut of. And since virDomainShutdown() is > > s/of/off/ > >> equivalent to calling virDomainShutdownFlags(0), it's up to each >> driver which methods to try. So the bare virDomainShutdown() may >> block or may return immediately at the same time. I know, it's >> confusing, but at least let users know. >> >> Signed-off-by: Michal Privoznik >> --- >> src/libvirt-domain.c | 13 + >> 1 file changed, 9 insertions(+), 4 deletions(-) >> > > I don't think this is the right approach. It is okay to block for a > small finite amount of time, but if the guest agent really is something > that can hang, we should fix that code to return without waiting for the > agent response. > The problem is, we can tell if the shutdown qemu-ga command succeeded only when we see the SHUTDOWN event on the domain monitor. The other option is to treat it like we're treating ACPI mode of the API: just send the request and report if the sending succeeded. Don't wait until domain actually shuts down. I can provide a patch. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemucaps2xmltest: fix test to successfully run without kvm support
Hi, On Wed, Mar 25, 2015 at 03:24:53PM +0100, Guido Günther wrote: > Hi, > On Wed, Mar 25, 2015 at 12:29:41PM +0100, Pavel Hrdina wrote: > > On Wed, Mar 25, 2015 at 12:12:20PM +0100, Ján Tomko wrote: > > > On Wed, Mar 25, 2015 at 10:50:52AM +0100, Pavel Hrdina wrote: > > > > Function virQEMUCapsInitGuestFromBinary detect kvm support by testing > > > > whether /dev/kvm exists or whether we pass path to kvmbin. Provide the > > > > path we are testing via kvmbin for testing purpose instead of detecting > > > > presence of /dev/kvm to successfully run the tests on all hosts. > > > > > > > > Signed-off-by: Pavel Hrdina > > > > --- > > > > tests/qemucaps2xmltest.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > ACK > > > > > > Jan > > > > Thanks, pushed now. > > According to > > Testbuild: > http://honk.sigxcpu.org:8001/job/libvirt-check/3300/consoleFull > SourceBuild > http://honk.sigxcpu.org:8001/job/libvirt-build/3730/ > > this broke on Debian Wheezy like: > > TEST: qemucaps2xmltest > 1) all_1.6.0-1 ... > Offset 280 > Expect [ > /usr/bin/qemu-system-i386 > > <] > Actual [<] > ... > FAILED > 2) nodisksnapshot_1.6.0-1... > Offset 280 > Expect [ > /usr/bin/qemu-system-i386 > > <] > Actual [<] > ... > FAILED > FAIL: qemucaps2xmltest > > I didn't get a chance to look into this in any detail yet. Any idea > what went wrong? Just for the record: this got fixed by eb05cb0d5c4ab013622a3695bcd60b72eeaa7dfe with logs at http://honk.sigxcpu.org:8001/job/libvirt-check/3324/ Cheers, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list