[Qemu-devel] [PATCH 1/5] hw/char: remove console_exit function in sclp

[Qemu-devel] [PATCH 1/5] hw/char: remove console_exit function in sclp

[Zihan Yang]

Currently, the console_exit function in sclpconsole-lm.c and sclpconsole.c
does nothing, so remove them and convert the callback in SCLPEventClass to
void. Since there is a NULL check on the DeviceClass exit callback as
suggested by Frederic Konrad, it should be safe to remove them.

Signed-off-by: Zihan Yang <tgnyang@gmail.com>
---
 hw/char/sclpconsole-lm.c          | 6 ------
 hw/char/sclpconsole.c             | 6 ------
 hw/s390x/event-facility.c         | 6 +-----
 include/hw/s390x/event-facility.h | 2 +-
 4 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index 07d6ebd..86ddda6 100644
--- a/hw/char/sclpconsole-lm.c
+++ b/hw/char/sclpconsole-lm.c
@@ -318,11 +318,6 @@ static int console_init(SCLPEvent *event)
     return 0;
 }
 
-static int console_exit(SCLPEvent *event)
-{
-    return 0;
-}
-
 static void console_reset(DeviceState *dev)
 {
    SCLPEvent *event = SCLP_EVENT(dev);
@@ -349,7 +344,6 @@ static void console_class_init(ObjectClass *klass, void *data)
     dc->reset = console_reset;
     dc->vmsd = &vmstate_sclplmconsole;
     ec->init = console_init;
-    ec->exit = console_exit;
     ec->get_send_mask = send_mask;
     ec->get_receive_mask = receive_mask;
     ec->can_handle_event = can_handle_event;
diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
index b78f240..e916cac 100644
--- a/hw/char/sclpconsole.c
+++ b/hw/char/sclpconsole.c
@@ -246,11 +246,6 @@ static void console_reset(DeviceState *dev)
    scon->notify = false;
 }
 
-static int console_exit(SCLPEvent *event)
-{
-    return 0;
-}
-
 static Property console_properties[] = {
     DEFINE_PROP_CHR("chardev", SCLPConsole, chr),
     DEFINE_PROP_END_OF_LIST(),
@@ -265,7 +260,6 @@ static void console_class_init(ObjectClass *klass, void *data)
     dc->reset = console_reset;
     dc->vmsd = &vmstate_sclpconsole;
     ec->init = console_init;
-    ec->exit = console_exit;
     ec->get_send_mask = send_mask;
     ec->get_receive_mask = receive_mask;
     ec->can_handle_event = can_handle_event;
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 34b2faf..f7c509c 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -413,11 +413,7 @@ static void event_unrealize(DeviceState *qdev, Error **errp)
     SCLPEvent *event = SCLP_EVENT(qdev);
     SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
     if (child->exit) {
-        int rc = child->exit(event);
-        if (rc < 0) {
-            error_setg(errp, "SCLP event exit failed.");
-            return;
-        }
+        child->exit(event);
     }
 }
 
diff --git a/include/hw/s390x/event-facility.h b/include/hw/s390x/event-facility.h
index def1bb0..1a32f3a 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -162,7 +162,7 @@ typedef struct SCLPEvent {
 typedef struct SCLPEventClass {
     DeviceClass parent_class;
     int (*init)(SCLPEvent *event);
-    int (*exit)(SCLPEvent *event);
+    void (*exit)(SCLPEvent *event);
 
     /* get SCLP's send mask */
     unsigned int (*get_send_mask)(void);
-- 
2.7.4

[Qemu-devel] [PATCH 2/5] hw/s390x: make virtio_ccw_exit function in virtio-ccw return void

[Zihan Yang]

Only virtio_ccw_exit and the callback in VirtIOCCWDeviceClass are
converted to void in this patch. 'virtio_ccw_busdev_exit' belongs to
DeviceClass so it still returns int at present, DeviceClass::exit
will return void in another patch.

Signed-off-by: Zihan Yang <tgnyang@gmail.com>
---
 hw/s390x/virtio-ccw.c | 6 +++---
 hw/s390x/virtio-ccw.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index e7167e3..91b43ac 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -731,7 +731,7 @@ out_err:
     g_free(sch);
 }
 
-static int virtio_ccw_exit(VirtioCcwDevice *dev)
+static void virtio_ccw_exit(VirtioCcwDevice *dev)
 {
     CcwDevice *ccw_dev = CCW_DEVICE(dev);
     SubchDev *sch = ccw_dev->sch;
@@ -744,7 +744,6 @@ static int virtio_ccw_exit(VirtioCcwDevice *dev)
         release_indicator(&dev->routes.adapter, dev->indicators);
         dev->indicators = NULL;
     }
-    return 0;
 }
 
 static void virtio_ccw_net_realize(VirtioCcwDevice *ccw_dev, Error **errp)
@@ -1627,7 +1626,8 @@ static int virtio_ccw_busdev_exit(DeviceState *dev)
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
     VirtIOCCWDeviceClass *_info = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
 
-    return _info->exit(_dev);
+    _info->exit(_dev);
+    return 0;
 }
 
 static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev,
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 41d4010..ce8baa3 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -74,7 +74,7 @@ typedef struct VirtioCcwDevice VirtioCcwDevice;
 typedef struct VirtIOCCWDeviceClass {
     CCWDeviceClass parent_class;
     void (*realize)(VirtioCcwDevice *dev, Error **errp);
-    int (*exit)(VirtioCcwDevice *dev);
+    void (*exit)(VirtioCcwDevice *dev);
 } VirtIOCCWDeviceClass;
 
 /* Performance improves when virtqueue kick processing is decoupled from the
-- 
2.7.4

[Qemu-devel] [PATCH 3/5] hw/s390: convert exit to unrealize in virtio_ccw_device_class_init

[Zihan Yang]

Currently the virtio_ccw_device_class_init function sets dc->exit, the
exit callback of DeviceClass. It should be converted to dc->unrealize
since exit callback of DeviceClass will be removed in the future.

Signed-off-by: Zihan Yang <tgnyang@gmail.com>
---
 hw/s390x/virtio-ccw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 91b43ac..5c193a8 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1621,13 +1621,12 @@ static void virtio_ccw_busdev_realize(DeviceState *dev, Error **errp)
     virtio_ccw_device_realize(_dev, errp);
 }
 
-static int virtio_ccw_busdev_exit(DeviceState *dev)
+static void virtio_ccw_busdev_unrealize(DeviceState *dev, Error **errp)
 {
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
     VirtIOCCWDeviceClass *_info = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
 
     _info->exit(_dev);
-    return 0;
 }
 
 static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev,
@@ -1645,7 +1644,7 @@ static void virtio_ccw_device_class_init(ObjectClass *klass, void *data)
 
     k->unplug = virtio_ccw_busdev_unplug;
     dc->realize = virtio_ccw_busdev_realize;
-    dc->exit = virtio_ccw_busdev_exit;
+    dc->unrealize = virtio_ccw_busdev_unrealize;
     dc->bus_type = TYPE_VIRTUAL_CSS_BUS;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH 4/5] hw/audio: replace exit with unrealize in hda_codec_device_class_init

[Zihan Yang]

The exit callback of DeviceClass will be removed in the future, so
convert to unrealize in the init functioin

Signed-off-by: Zihan Yang <tgnyang@gmail.com>
---
 hw/audio/intel-hda.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 537face..2c497eb 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -70,7 +70,7 @@ static void hda_codec_dev_realize(DeviceState *qdev, Error **errp)
     }
 }
 
-static int hda_codec_dev_exit(DeviceState *qdev)
+static void hda_codec_dev_unrealize(DeviceState *qdev, Error **errp)
 {
     HDACodecDevice *dev = HDA_CODEC_DEVICE(qdev);
     HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
@@ -78,7 +78,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
     if (cdc->exit) {
         cdc->exit(dev);
     }
-    return 0;
 }
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
@@ -1318,7 +1317,7 @@ static void hda_codec_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->realize = hda_codec_dev_realize;
-    k->exit = hda_codec_dev_exit;
+    k->unrealize = hda_codec_dev_unrealize;
     set_bit(DEVICE_CATEGORY_SOUND, k->categories);
     k->bus_type = TYPE_HDA_BUS;
     k->props = hda_props;
-- 
2.7.4

[Qemu-devel] [PATCH 5/5] hw/audio: convert exit callback in HDACodecDeviceClass to void

[Zihan Yang]

The exit callback always return 0, convert it to void

Signed-off-by: Zihan Yang <tgnyang@gmail.com>
---
 hw/audio/hda-codec.c | 3 +--
 hw/audio/intel-hda.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 52d4640..5402cd1 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
     return 0;
 }
 
-static int hda_audio_exit(HDACodecDevice *hda)
+static void hda_audio_exit(HDACodecDevice *hda)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
     HDAAudioStream *st;
@@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
         }
     }
     AUD_remove_card(&a->card);
-    return 0;
 }
 
 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index d784bcf..53b78da 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
     DeviceClass parent_class;
 
     int (*init)(HDACodecDevice *dev);
-    int (*exit)(HDACodecDevice *dev);
+    void (*exit)(HDACodecDevice *dev);
     void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
     void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool output);
 } HDACodecDeviceClass;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/5] hw/char: remove console_exit function in sclp

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

On 04/26/2017 07:53 AM, Zihan Yang wrote:
> Currently, the console_exit function in sclpconsole-lm.c and sclpconsole.c
> does nothing, so remove them and convert the callback in SCLPEventClass to
> void. Since there is a NULL check on the DeviceClass exit callback as
> suggested by Frederic Konrad, it should be safe to remove them.

When sending a patch series, please be sure to include a 0/5 cover
letter, to make it easier on automated tooling that processes the list.
You can use 'git config format.coverletter auto' to make it easier to
remember this for patches created by 'git format-patch'/'git send-email'.

More submission hints at: http://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/5] hw/char: remove console_exit function in sclp

[Cornelia Huck]

On Wed, 26 Apr 2017 09:42:02 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 04/26/2017 07:53 AM, Zihan Yang wrote:
> > Currently, the console_exit function in sclpconsole-lm.c and sclpconsole.c
> > does nothing, so remove them and convert the callback in SCLPEventClass to
> > void. Since there is a NULL check on the DeviceClass exit callback as
> > suggested by Frederic Konrad, it should be safe to remove them.
> 
> When sending a patch series, please be sure to include a 0/5 cover
> letter, to make it easier on automated tooling that processes the list.
> You can use 'git config format.coverletter auto' to make it easier to
> remember this for patches created by 'git format-patch'/'git send-email'.
> 
> More submission hints at: http://wiki.qemu.org/Contribute/SubmitAPatch
> 

Nod.

Of the hints in the wiki page above, I'd especially like to point out
versioning and change logs.

Re: [Qemu-devel] [PATCH 1/5] hw/char: remove console_exit function in sclp

[Zihan Yang]

OK, sorry for the confusion, I will give a new patch series. I'm not very
familiar with
the conventions so I wonder if someone could help clarify some principles
for me.
I'd like to replace all the init/exit callback of DeviceClass to
realize/unrealize,  and
convert return type of exit callback of some higher-level classes, like
HDACodecDeviceClass, to void.

My first question Is it a good idea to split these patches into two series?
For example,
one series to convert exit callback of these higher-level classes to void,
and then the
other to replace all the init/exit callback of DeviceClass to
realize/unrealize.

The second question is that should I always give separate patch for
different directories?
One  example is that in both hw/ide and hw/block, I need to replace the
init callback with
realize in some high-level classes, should I give two patches or just give
one patch for
the work?

Thanks

Re: [Qemu-devel] [PATCH 1/5] hw/char: remove console_exit function in sclp

[Cornelia Huck]

On Thu, 27 Apr 2017 20:23:51 +0800
Zihan Yang <tgnyang@gmail.com> wrote:

> OK, sorry for the confusion, I will give a new patch series. I'm not very
> familiar with
> the conventions so I wonder if someone could help clarify some principles
> for me.
> I'd like to replace all the init/exit callback of DeviceClass to
> realize/unrealize,  and
> convert return type of exit callback of some higher-level classes, like
> HDACodecDeviceClass, to void.
> 
> My first question Is it a good idea to split these patches into two series?
> For example,
> one series to convert exit callback of these higher-level classes to void,
> and then the
> other to replace all the init/exit callback of DeviceClass to
> realize/unrealize.

This is probably a good idea: converting exit callbacks to void is
relatively straightforward, while converting to realize/unrealize might
be better done while revisiting some of the modelling of the relevant
subsystems. (For example, all classes for virtio-ccw use the same exit
callback - it might be a good idea to do the same work in a common
unrealize function when you touch it anyway.)

> 
> The second question is that should I always give separate patch for
> different directories?
> One  example is that in both hw/ide and hw/block, I need to replace the
> init callback with
> realize in some high-level classes, should I give two patches or just give
> one patch for
> the work?

A better way to split is along maintainership responsibilites (which
may or may not align with directories). Again, sticking with the code I
maintain, I would merge changes to hw/char/sclp* and hw/s390x/, but not
to other code in hw/char/. Try to stick to logical units (continuing
with that example, different patches for sclp and virtio-ccw would make
sense).

The most important thing to remember when splitting changes is that you
need to preserve bisectability, i.e. every step in your patch series
needs to compile.