Re: [PATCH v2 6/8] qemuDomainEventQueue: Check if event is non-NULL

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

 



On Thu, Sep 17, 2015 at 18:24:29 -0400, John Ferlan wrote:
> 
> 
> On 09/11/2015 09:26 AM, Jiri Denemark wrote:
> > Every single call to qemuDomainEventQueue() uses the following pattern:
> > 
> >     if (event)
> >         qemuDomainEventQueue(driver, event);
> > 
> > Let's move the check for valid event to qemuDomainEventQueue and
> > simplify all callers.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> > ---
> > 
> > Notes:
> >     Version 2:
> >     - no change
> > 
> >  src/qemu/qemu_blockjob.c  |  6 ++--
> >  src/qemu/qemu_cgroup.c    |  3 +-
> >  src/qemu/qemu_domain.c    |  6 ++--
> >  src/qemu/qemu_driver.c    | 87 ++++++++++++++++-------------------------------
> >  src/qemu/qemu_hotplug.c   | 26 ++++++--------
> >  src/qemu/qemu_migration.c | 24 +++++--------
> >  src/qemu/qemu_process.c   | 72 +++++++++++++--------------------------
> >  7 files changed, 78 insertions(+), 146 deletions(-)
> > 
> 
> Yay!  A couple still are behind "if (event) {" in qemu_driver
> (qemuDomainCreateXML, qemuDomainRevertToSnapshot) as well as an if

There are two events involved in both functions ("event" and "event2")
and we can send event2 only if event is not NULL, which is the reason
for
    if (event) {
        qemuDomainEventQueue(driver, event);
        qemuDomainEventQueue(driver, event2);
    }

It would be possible to rewrite it as

    qemuDomainEventQueue(driver, event);
    if (event)
        qemuDomainEventQueue(driver, event2);

but I think the original version is slightly better.

> (event_old) and if (event_new) in qemuDomainRename

Yeah, both came in after I originally created this patch. I will clean
them before pushing.

Jirka

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]