[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 00/37] Change OOM reporting



On 04/29/2013 09:50 AM, Michal Privoznik wrote:
> Previously, the first version dropped virReportOOMError from
> almost everywhere. This series don't set such high goal yet. It
> just prepares ground for doing so. The worst case scenario is the
> OOM error is reported twice. Once in VIR_ALLOC, VIR_STRDUP,
> virAsprintf (or similar), then in virReportOOMError itself.
> However, once this is pushed, it's easy to get rid of the latter
> one.
> 

I've gone through each of the patches and figured I'd do one synopsis
message!

>From the code motion and adjustment, it seems things are OK. I tried to
keep focus, but the monotony of motion could have just as easily played
tricks on my eyes.

I noted a few issues that need cleanup (the missing increment in 12, an
'm' instead of a ',', a couple of spacing things, and the syntax-check
STRUP typo.

While not called out, patches 24 (test), 26 (util), and 30 (xen) also
had a few instances of auto increment inside the STR*DUP macro. It seems
from Eric's comments from 12 that it's OK to do so as long as it's
documented that way.

Beyond that - thanks for a new way to take power naps :-)

John

> Patches available at:
>   git://gitorious.org/~zippy2/libvirt/michal-staging.git
>   branch oom3
> 
> Remember, this will not be pushed until we are after the freeze.
> But the freeze creates a nice steady HEAD which is needed in such
> a giant change like this one.
> 
> Michal Privoznik (37):
>   virutil: Move string related functions to virstring.c
>   virstring: Introduce VIR_STRUP and VIR_STRNDUP
>   Adapt to VIR_STRDUP in daemon/*
>   Adapt to VIR_STRDUP in src/conf/*
>   Adapt to VIR_STRDUP in src/cpu/*
>   Adapt to VIR_STRDUP in src/esx/*
>   Adapt to VIR_STRDUP in src/hyperv/*
>   Adapt to VIR_STRDUP in src/libxl/*
>   Adapt to VIR_STRDUP in src/locking/*
>   Adapt to VIR_STRDUP in src/lxc/*
>   Adapt to VIR_STRDUP in src/network/*
>   Adapt to VIR_STRDUP in src/node_device/*
>   Adapt to VIR_STRDUP in src/interface/
>   Adapt to VIR_STRDUP in src/nwfilter/*
>   Adapt to VIR_STRDUP in src/openvz/*
>   Adapt to VIR_STRDUP in src/parallels/*
>   Adapt to VIR_STRDUP in src/phyp/*
>   Adapt to VIR_STRDUP in src/qemu/*
>   Adapt to VIR_STRDUP in src/remote/*
>   Adapt to VIR_STRDUP in src/rpc/*
>   Adapt to VIR_STRDUP in src/secret/*
>   Adapt to VIR_STRDUP in src/security/*
>   Adapt to VIR_STRDUP in src/storage/*
>   Adapt to VIR_STRDUP in src/test/*
>   Adapt to VIR_STRDUP in src/uml/*
>   Adapt to VIR_STRDUP in src/util/*
>   Adapt to VIR_STRDUP in src/vbox/*
>   Adapt to VIR_STRDUP in src/vmware/*
>   Adapt to VIR_STRDUP in src/vmx/*
>   Adapt to VIR_STRDUP in src/xen/*
>   Adapt to VIR_STRDUP in src/xenapi/*
>   Adapt to VIR_STRDUP in src/xenxs/*
>   Adapt to VIR_STRDUP in tests/*
>   Adapt to VIR_STRDUP in src/*
>   Introduce syntax-check rule to prefer VIR_STRDUP over strdup
>   virstring: Introduce virAsprintfQuiet and virVasprintfQuiet
>   viralloc.c: Do report OOM error in VIR_ALLOC and friends
> 
>  HACKING                                   |  19 ++
>  cfg.mk                                    |  12 +-
>  daemon/libvirtd-config.c                  |  39 +--
>  daemon/libvirtd.c                         |  19 +-
>  daemon/remote.c                           |  97 +++----
>  docs/hacking.html.in                      |  26 ++
>  include/libvirt/libvirt.h.in              |  10 +-
>  po/POTFILES.in                            |   1 +
>  python/libvirt-override.c                 | 103 +++----
>  src/conf/capabilities.c                   |  27 +-
>  src/conf/cpu_conf.c                       |  14 +-
>  src/conf/device_conf.c                    |   2 +-
>  src/conf/domain_audit.c                   |   1 +
>  src/conf/domain_conf.c                    |  66 ++---
>  src/conf/domain_conf.h                    |   1 -
>  src/conf/domain_event.c                   |  31 +-
>  src/conf/interface_conf.c                 |   1 -
>  src/conf/netdev_bandwidth_conf.c          |   2 +-
>  src/conf/netdev_vport_profile_conf.c      |   1 +
>  src/conf/network_conf.c                   |   2 +-
>  src/conf/node_device_conf.c               |  29 +-
>  src/conf/nwfilter_conf.c                  |  20 +-
>  src/conf/nwfilter_conf.h                  |   1 -
>  src/conf/nwfilter_params.c                |  23 +-
>  src/conf/secret_conf.c                    |   1 -
>  src/conf/snapshot_conf.c                  |   8 +-
>  src/conf/storage_conf.c                   |  11 +-
>  src/conf/storage_conf.h                   |   1 -
>  src/conf/storage_encryption_conf.c        |   1 -
>  src/conf/virchrdev.c                      |   7 +-
>  src/cpu/cpu_generic.c                     |   6 +-
>  src/cpu/cpu_map.c                         |   3 +-
>  src/cpu/cpu_powerpc.c                     |  11 +-
>  src/cpu/cpu_x86.c                         |   9 +-
>  src/datatypes.c                           |  26 +-
>  src/driver.c                              |   2 +-
>  src/esx/esx_device_monitor.c              |   1 -
>  src/esx/esx_driver.c                      |  35 +--
>  src/esx/esx_interface_driver.c            |   6 +-
>  src/esx/esx_network_driver.c              |  21 +-
>  src/esx/esx_nwfilter_driver.c             |   1 -
>  src/esx/esx_secret_driver.c               |   1 -
>  src/esx/esx_storage_backend_iscsi.c       |  15 +-
>  src/esx/esx_storage_backend_vmfs.c        |  10 +-
>  src/esx/esx_util.c                        |  39 +--
>  src/esx/esx_vi.c                          |  54 +---
>  src/esx/esx_vi_types.c                    |  24 +-
>  src/fdstream.c                            |   1 +
>  src/hyperv/hyperv_device_monitor.c        |   1 -
>  src/hyperv/hyperv_driver.c                |  37 +--
>  src/hyperv/hyperv_interface_driver.c      |   1 -
>  src/hyperv/hyperv_network_driver.c        |   1 -
>  src/hyperv/hyperv_nwfilter_driver.c       |   1 -
>  src/hyperv/hyperv_secret_driver.c         |   1 -
>  src/hyperv/hyperv_storage_driver.c        |   1 -
>  src/hyperv/hyperv_util.c                  |  16 +-
>  src/hyperv/hyperv_wmi.c                   |   2 +-
>  src/interface/interface_backend_udev.c    |  25 +-
>  src/libvirt.c                             |  10 +-
>  src/libvirt_private.syms                  |  36 +--
>  src/libxl/libxl_conf.c                    |  59 ++--
>  src/libxl/libxl_driver.c                  |  14 +-
>  src/locking/lock_daemon.c                 |  13 +-
>  src/locking/lock_daemon_config.c          |  10 +-
>  src/locking/lock_daemon_dispatch.c        |   4 +-
>  src/locking/lock_driver_lockd.c           |  24 +-
>  src/locking/lock_driver_sanlock.c         |   9 +-
>  src/locking/lock_manager.c                |   4 +-
>  src/lxc/lxc_cgroup.c                      |   3 +-
>  src/lxc/lxc_conf.c                        |  18 +-
>  src/lxc/lxc_container.c                   |  10 +-
>  src/lxc/lxc_controller.c                  |   8 +-
>  src/lxc/lxc_driver.c                      |   9 +-
>  src/lxc/lxc_fuse.c                        |   1 +
>  src/lxc/lxc_fuse.h                        |   1 -
>  src/lxc/lxc_monitor.c                     |   1 +
>  src/lxc/lxc_process.c                     |   5 +-
>  src/network/bridge_driver.c               |  30 +-
>  src/node_device/node_device_driver.c      |  29 +-
>  src/node_device/node_device_hal.c         |  13 +-
>  src/node_device/node_device_linux_sysfs.c |   1 +
>  src/node_device/node_device_udev.c        | 104 +++----
>  src/nodeinfo.c                            |   6 +-
>  src/nwfilter/nwfilter_dhcpsnoop.c         |  16 +-
>  src/nwfilter/nwfilter_driver.c            |   5 +-
>  src/nwfilter/nwfilter_ebiptables_driver.c |  12 +-
>  src/nwfilter/nwfilter_gentech_driver.c    |   4 +-
>  src/nwfilter/nwfilter_learnipaddr.c       |   4 +-
>  src/openvz/openvz_conf.c                  |  33 ++-
>  src/openvz/openvz_driver.c                |  32 +--
>  src/parallels/parallels_driver.c          |  36 +--
>  src/parallels/parallels_network.c         |  14 +-
>  src/parallels/parallels_storage.c         |  40 ++-
>  src/parallels/parallels_utils.c           |   2 +-
>  src/phyp/phyp_driver.c                    |  37 +--
>  src/qemu/qemu_agent.c                     |   1 +
>  src/qemu/qemu_bridge_filter.c             |   1 -
>  src/qemu/qemu_capabilities.c              |  38 +--
>  src/qemu/qemu_cgroup.c                    |   4 +-
>  src/qemu/qemu_command.c                   | 282 ++++++++-----------
>  src/qemu/qemu_conf.c                      |  33 +--
>  src/qemu/qemu_domain.c                    |  17 +-
>  src/qemu/qemu_driver.c                    |  93 +++---
>  src/qemu/qemu_hotplug.c                   |   9 +-
>  src/qemu/qemu_migration.c                 |  17 +-
>  src/qemu/qemu_monitor.c                   |   1 +
>  src/qemu/qemu_monitor_json.c              |  40 ++-
>  src/qemu/qemu_monitor_text.c              |  10 +-
>  src/qemu/qemu_process.c                   |  58 ++--
>  src/remote/remote_driver.c                |  83 +++---
>  src/rpc/gendispatch.pl                    |  13 +-
>  src/rpc/virkeepalive.c                    |   1 -
>  src/rpc/virnetclient.c                    |   8 +-
>  src/rpc/virnetmessage.c                   |  11 +-
>  src/rpc/virnetsaslcontext.c               |   3 +-
>  src/rpc/virnetserver.c                    |   4 +-
>  src/rpc/virnetserverclient.c              |   8 +-
>  src/rpc/virnetservermdns.c                |   4 +-
>  src/rpc/virnetsocket.c                    |   8 +-
>  src/rpc/virnetsshsession.c                |  23 +-
>  src/rpc/virnettlscontext.c                |  14 +-
>  src/secret/secret_driver.c                |   5 +-
>  src/security/security_apparmor.c          |  13 +-
>  src/security/security_dac.c               |   8 +-
>  src/security/security_nop.c               |   4 +-
>  src/security/security_selinux.c           |  51 ++--
>  src/security/virt-aa-helper.c             |   6 +-
>  src/storage/parthelper.c                  |   7 +-
>  src/storage/storage_backend.c             |  18 +-
>  src/storage/storage_backend_disk.c        |  16 +-
>  src/storage/storage_backend_fs.c          |  18 +-
>  src/storage/storage_backend_iscsi.c       |  16 +-
>  src/storage/storage_backend_logical.c     |  25 +-
>  src/storage/storage_backend_mpath.c       |   4 +-
>  src/storage/storage_backend_rbd.c         |   4 +-
>  src/storage/storage_backend_scsi.c        |  17 +-
>  src/storage/storage_backend_sheepdog.c    |   1 -
>  src/storage/storage_driver.c              |  13 +-
>  src/test/test_driver.c                    |  84 +++---
>  src/uml/uml_conf.c                        |   7 +-
>  src/uml/uml_driver.c                      |   8 +-
>  src/util/iohelper.c                       |   1 +
>  src/util/viralloc.c                       |  95 ++++++-
>  src/util/viralloc.h                       |  54 +++-
>  src/util/viraudit.c                       |   6 +-
>  src/util/virauth.c                        |   9 +-
>  src/util/virauthconfig.c                  |   7 +-
>  src/util/virbitmap.c                      |  10 +-
>  src/util/virbuffer.c                      |   8 +-
>  src/util/vircgroup.c                      |  31 +-
>  src/util/vircommand.c                     |  34 ++-
>  src/util/vircommand.h                     |   1 -
>  src/util/virconf.c                        |  21 +-
>  src/util/virdnsmasq.c                     |   5 +-
>  src/util/virebtables.c                    |  36 +--
>  src/util/virerror.c                       |  25 +-
>  src/util/virfile.c                        |   1 +
>  src/util/virhash.c                        |   5 +-
>  src/util/virhook.h                        |   1 -
>  src/util/viridentity.c                    |   9 +-
>  src/util/virinitctl.c                     |   3 +-
>  src/util/viriptables.c                    |   6 +-
>  src/util/virjson.c                        |  18 +-
>  src/util/virkeyfile.c                     |   7 +-
>  src/util/virlockspace.c                   |  14 +-
>  src/util/virlog.c                         |  21 +-
>  src/util/virnetdev.c                      |   2 +
>  src/util/virnetdevbandwidth.c             |   1 +
>  src/util/virnetdevbridge.c                |   1 +
>  src/util/virnetdevmacvlan.c               |  10 +-
>  src/util/virnetdevopenvswitch.c           |   1 +
>  src/util/virnetdevopenvswitch.h           |   1 -
>  src/util/virnetdevtap.c                   |   7 +-
>  src/util/virnetdevveth.c                  |   2 +
>  src/util/virnetdevvportprofile.c          |   4 +-
>  src/util/virobject.c                      |   5 +-
>  src/util/virpci.c                         |   7 +-
>  src/util/virpidfile.c                     |   1 +
>  src/util/virprocess.c                     |   1 +
>  src/util/virrandom.c                      |   1 +
>  src/util/virsexpr.c                       |  27 +-
>  src/util/virsocketaddr.c                  |   6 +-
>  src/util/virstatslinux.c                  |   1 -
>  src/util/virstoragefile.c                 |  13 +-
>  src/util/virstring.c                      | 451 +++++++++++++++++++++++++++++-
>  src/util/virstring.h                      |  74 +++++
>  src/util/virsysinfo.c                     | 170 +++++------
>  src/util/virthreadpthread.c               |   2 +-
>  src/util/virtime.c                        |   1 -
>  src/util/virtpm.c                         |   2 +-
>  src/util/virtypedparam.c                  |   9 +-
>  src/util/viruri.c                         |  20 +-
>  src/util/virusb.c                         |   1 +
>  src/util/virutil.c                        | 393 ++------------------------
>  src/util/virutil.h                        |  49 ----
>  src/util/virxml.c                         |   7 +-
>  src/vbox/vbox_MSCOMGlue.c                 |   1 -
>  src/vbox/vbox_XPCOMCGlue.c                |   5 +-
>  src/vbox/vbox_driver.c                    |   1 -
>  src/vbox/vbox_tmpl.c                      | 191 +++++++------
>  src/vmware/vmware_conf.c                  |  15 +-
>  src/vmware/vmware_driver.c                |  13 +-
>  src/vmx/vmx.c                             |  26 +-
>  src/xen/block_stats.c                     |   2 +-
>  src/xen/xen_driver.c                      |   4 +-
>  src/xen/xen_hypervisor.c                  |  17 +-
>  src/xen/xen_inotify.c                     |  11 +-
>  src/xen/xend_internal.c                   |  55 ++--
>  src/xen/xm_internal.c                     |   8 +-
>  src/xen/xs_internal.c                     |  22 +-
>  src/xenapi/xenapi_driver.c                |  34 ++-
>  src/xenapi/xenapi_utils.c                 |  22 +-
>  src/xenxs/xen_sxpr.c                      | 108 +++----
>  src/xenxs/xen_xm.c                        |  44 +--
>  tests/commandhelper.c                     |  14 +-
>  tests/commandtest.c                       |   4 +-
>  tests/cputest.c                           |   1 +
>  tests/domainsnapshotxml2xmltest.c         |   1 +
>  tests/esxutilstest.c                      |   1 -
>  tests/interfacexml2xmltest.c              |   2 +-
>  tests/libvirtdconftest.c                  |   1 +
>  tests/lxcxml2xmltest.c                    |   1 +
>  tests/networkxml2conftest.c               |   3 +
>  tests/networkxml2xmltest.c                |   1 +
>  tests/nodedevxml2xmltest.c                |   1 +
>  tests/nodeinfotest.c                      |   2 +-
>  tests/nwfilterxml2xmltest.c               |   1 +
>  tests/openvzutilstest.c                   |   8 +-
>  tests/qemuargv2xmltest.c                  |   2 +-
>  tests/qemuhelptest.c                      |   1 +
>  tests/qemumonitortest.c                   |   1 -
>  tests/qemumonitortestutils.c              |  10 +-
>  tests/qemuxml2argvtest.c                  |  28 +-
>  tests/qemuxml2xmltest.c                   |   1 +
>  tests/qemuxmlnstest.c                     |   6 +-
>  tests/securityselinuxhelper.c             |   7 +-
>  tests/securityselinuxlabeltest.c          |   5 +-
>  tests/securityselinuxtest.c               |   9 +-
>  tests/sexpr2xmltest.c                     |   1 +
>  tests/storagebackendsheepdogtest.c        |   6 +-
>  tests/storagepoolxml2xmltest.c            |   1 +
>  tests/storagevolxml2argvtest.c            |   1 +
>  tests/storagevolxml2xmltest.c             |   1 +
>  tests/sysinfotest.c                       |   2 +-
>  tests/test_conf.c                         |   2 +-
>  tests/testutils.c                         |   5 +-
>  tests/testutilsqemu.c                     |   5 +-
>  tests/virauthconfigtest.c                 |   1 -
>  tests/virbuftest.c                        |   2 +-
>  tests/vircgrouptest.c                     |   4 +-
>  tests/virdrivermoduletest.c               |   1 -
>  tests/virhashtest.c                       |   3 +-
>  tests/viridentitytest.c                   |   1 -
>  tests/virkeyfiletest.c                    |   1 -
>  tests/virnetmessagetest.c                 |  22 +-
>  tests/virnetsockettest.c                  |   1 +
>  tests/virportallocatortest.c              |   2 +-
>  tests/virshtest.c                         |   2 +-
>  tests/virstoragetest.c                    |   1 +
>  tests/virstringtest.c                     |   1 -
>  tests/virtimetest.c                       |   1 -
>  tests/viruritest.c                        |   1 -
>  tests/vmx2xmltest.c                       |  11 +-
>  tests/xencapstest.c                       |   1 +
>  tests/xmconfigtest.c                      |   3 +
>  tests/xml2sexprtest.c                     |   1 +
>  tests/xml2vmxtest.c                       |  11 +-
>  tools/console.c                           |   1 -
>  tools/virsh-domain-monitor.c              |   1 +
>  tools/virsh-domain.c                      |   1 -
>  tools/virsh-host.c                        |   2 +-
>  tools/virsh-interface.c                   |   1 +
>  tools/virsh-network.c                     |   1 -
>  tools/virsh-nodedev.c                     |   1 -
>  tools/virsh-pool.c                        |   2 +-
>  tools/virsh-snapshot.c                    |   1 -
>  tools/virsh-volume.c                      |   1 +
>  tools/virsh.c                             |   8 +-
>  tools/virt-host-validate-common.c         |   1 +
>  279 files changed, 2544 insertions(+), 2653 deletions(-)
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]