* [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize @ 2017-05-26 12:15 Mao Zhongyi 2017-05-26 14:08 ` Marcel Apfelbaum 0 siblings, 1 reply; 6+ messages in thread From: Mao Zhongyi @ 2017-05-26 12:15 UTC (permalink / raw) To: qemu-devel; +Cc: marcel, mst, armbru The pci-birdge device i82801b11 still implements the old PCIDeviceClass .init() through i82801b11_bridge_init() instead of the new .realize(). All devices need to be converted to .realize(). So convert it and rename it to i82801b11_bridge_realize(). Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> --- hw/pci-bridge/i82801b11.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c index 2404e7e..dca3162 100644 --- a/hw/pci-bridge/i82801b11.c +++ b/hw/pci-bridge/i82801b11.c @@ -44,6 +44,7 @@ #include "qemu/osdep.h" #include "hw/pci/pci.h" #include "hw/i386/ich9.h" +#include "qapi/error.h" /*****************************************************************************/ @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge { /*< public >*/ } I82801b11Bridge; -static int i82801b11_bridge_initfn(PCIDevice *d) +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp) { int rc; @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d) goto err_bridge; } pci_config_set_prog_interface(d->config, PCI_CLASS_BRIDGE_PCI_INF_SUB); - return 0; + return; err_bridge: pci_bridge_exitfn(d); - - return rc; } static const VMStateDescription i82801b11_bridge_dev_vmstate = { @@ -95,7 +94,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_INTEL; k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11; k->revision = ICH9_D2P_A2_REVISION; - k->init = i82801b11_bridge_initfn; + k->realize = i82801b11_bridge_realize; k->config_write = pci_bridge_write_config; dc->vmsd = &i82801b11_bridge_dev_vmstate; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); -- 2.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize 2017-05-26 12:15 [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize Mao Zhongyi @ 2017-05-26 14:08 ` Marcel Apfelbaum 2017-05-27 6:58 ` Mao Zhongyi 0 siblings, 1 reply; 6+ messages in thread From: Marcel Apfelbaum @ 2017-05-26 14:08 UTC (permalink / raw) To: Mao Zhongyi, qemu-devel; +Cc: mst, armbru On 26/05/2017 15:15, Mao Zhongyi wrote: > The pci-birdge device i82801b11 still implements the old > PCIDeviceClass .init() through i82801b11_bridge_init() > instead of the new .realize(). All devices need to be > converted to .realize(). So convert it and rename it to > i82801b11_bridge_realize(). > > Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> > --- > hw/pci-bridge/i82801b11.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c > index 2404e7e..dca3162 100644 > --- a/hw/pci-bridge/i82801b11.c > +++ b/hw/pci-bridge/i82801b11.c > @@ -44,6 +44,7 @@ > #include "qemu/osdep.h" > #include "hw/pci/pci.h" > #include "hw/i386/ich9.h" > +#include "qapi/error.h" > > > /*****************************************************************************/ > @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge { > /*< public >*/ > } I82801b11Bridge; > > -static int i82801b11_bridge_initfn(PCIDevice *d) > +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp) > { > int rc; > > @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d) > goto err_bridge; > } > pci_config_set_prog_interface(d->config, PCI_CLASS_BRIDGE_PCI_INF_SUB); > - return 0; > + return; > > err_bridge: > pci_bridge_exitfn(d); Hi, Since you move to realize, you may want to leverage the errp to add info on errors. Thanks, Marcel > - > - return rc; > } > > static const VMStateDescription i82801b11_bridge_dev_vmstate = { > @@ -95,7 +94,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data) > k->vendor_id = PCI_VENDOR_ID_INTEL; > k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11; > k->revision = ICH9_D2P_A2_REVISION; > - k->init = i82801b11_bridge_initfn; > + k->realize = i82801b11_bridge_realize; > k->config_write = pci_bridge_write_config; > dc->vmsd = &i82801b11_bridge_dev_vmstate; > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize 2017-05-26 14:08 ` Marcel Apfelbaum @ 2017-05-27 6:58 ` Mao Zhongyi 2017-05-27 17:01 ` Marcel Apfelbaum 0 siblings, 1 reply; 6+ messages in thread From: Mao Zhongyi @ 2017-05-27 6:58 UTC (permalink / raw) To: Marcel Apfelbaum, qemu-devel; +Cc: mst, armbru On 05/26/2017 10:08 PM, Marcel Apfelbaum wrote: > > > On 26/05/2017 15:15, Mao Zhongyi wrote: >> The pci-birdge device i82801b11 still implements the old >> PCIDeviceClass .init() through i82801b11_bridge_init() >> instead of the new .realize(). All devices need to be >> converted to .realize(). So convert it and rename it to >> i82801b11_bridge_realize(). >> >> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> >> --- >> hw/pci-bridge/i82801b11.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c >> index 2404e7e..dca3162 100644 >> --- a/hw/pci-bridge/i82801b11.c >> +++ b/hw/pci-bridge/i82801b11.c >> @@ -44,6 +44,7 @@ >> #include "qemu/osdep.h" >> #include "hw/pci/pci.h" >> #include "hw/i386/ich9.h" >> +#include "qapi/error.h" >> /*****************************************************************************/ >> @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge { >> /*< public >*/ >> } I82801b11Bridge; >> -static int i82801b11_bridge_initfn(PCIDevice *d) >> +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp) >> { >> int rc; >> @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d) >> goto err_bridge; >> } >> pci_config_set_prog_interface(d->config, PCI_CLASS_BRIDGE_PCI_INF_SUB); >> - return 0; >> + return; >> err_bridge: >> pci_bridge_exitfn(d); > > Hi, > > Since you move to realize, you may want to leverage the errp to add > info on errors. > > Thanks, > Marcel Hi, Marcel Thanks for your quick reply and advice. In fact, I have considered adding an error message to errp when an error occurs. But I found that pci_bridge_ssvid_init() has reported a specific info when it fails. If the error is reported here again, it seems a bit more superfluous, so it's not adopted. Of course, output a readable error info to make it easier for users is also a good choice. So, are you sure you want to do that? Look forward to your feedback and suggestion soon. Thanks a lot. Mao > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize 2017-05-27 6:58 ` Mao Zhongyi @ 2017-05-27 17:01 ` Marcel Apfelbaum 2017-05-29 11:37 ` Markus Armbruster 0 siblings, 1 reply; 6+ messages in thread From: Marcel Apfelbaum @ 2017-05-27 17:01 UTC (permalink / raw) To: Mao Zhongyi, qemu-devel; +Cc: mst, armbru On 27/05/2017 9:58, Mao Zhongyi wrote: > > > On 05/26/2017 10:08 PM, Marcel Apfelbaum wrote: >> >> >> On 26/05/2017 15:15, Mao Zhongyi wrote: >>> The pci-birdge device i82801b11 still implements the old >>> PCIDeviceClass .init() through i82801b11_bridge_init() >>> instead of the new .realize(). All devices need to be >>> converted to .realize(). So convert it and rename it to >>> i82801b11_bridge_realize(). >>> >>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> >>> --- >>> hw/pci-bridge/i82801b11.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c >>> index 2404e7e..dca3162 100644 >>> --- a/hw/pci-bridge/i82801b11.c >>> +++ b/hw/pci-bridge/i82801b11.c >>> @@ -44,6 +44,7 @@ >>> #include "qemu/osdep.h" >>> #include "hw/pci/pci.h" >>> #include "hw/i386/ich9.h" >>> +#include "qapi/error.h" >>> /*****************************************************************************/ >>> @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge { >>> /*< public >*/ >>> } I82801b11Bridge; >>> -static int i82801b11_bridge_initfn(PCIDevice *d) >>> +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp) >>> { >>> int rc; >>> @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d) >>> goto err_bridge; >>> } >>> pci_config_set_prog_interface(d->config, >>> PCI_CLASS_BRIDGE_PCI_INF_SUB); >>> - return 0; >>> + return; >>> err_bridge: >>> pci_bridge_exitfn(d); >> >> Hi, >> >> Since you move to realize, you may want to leverage the errp to add >> info on errors. >> >> Thanks, >> Marcel > > > Hi, Marcel > Hi! > Thanks for your quick reply and advice. In fact, I have considered > adding an error > message to errp when an error occurs. But I found that > pci_bridge_ssvid_init() has > reported a specific info when it fails. If the error is reported here > again, it seems > a bit more superfluous, so it's not adopted. I agree we don't want to see the error twice, but that means we may want to pass the error pointer to pci_bridge_ssvid_init and so on. One of the main things we achieve when moving to 'realize' is better error handling, so if we don't do that maybe is not worth it. You may need to do several changes you didn't intend to in order to do the error propagation right... Thanks, Marcel > Of course, output a readable error info > to make it easier for users is also a good choice. So, are you sure > you want to do > that? > > Look forward to your feedback and suggestion soon. > > Thanks a lot. > Mao > > > > > > > > > > > > > > > > > > > >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize 2017-05-27 17:01 ` Marcel Apfelbaum @ 2017-05-29 11:37 ` Markus Armbruster 2017-05-31 1:46 ` Mao Zhongyi 0 siblings, 1 reply; 6+ messages in thread From: Markus Armbruster @ 2017-05-29 11:37 UTC (permalink / raw) To: Marcel Apfelbaum; +Cc: Mao Zhongyi, qemu-devel, mst Marcel Apfelbaum <marcel@redhat.com> writes: > On 27/05/2017 9:58, Mao Zhongyi wrote: >> >> >> On 05/26/2017 10:08 PM, Marcel Apfelbaum wrote: >>> >>> >>> On 26/05/2017 15:15, Mao Zhongyi wrote: >>>> The pci-birdge device i82801b11 still implements the old >>>> PCIDeviceClass .init() through i82801b11_bridge_init() >>>> instead of the new .realize(). All devices need to be >>>> converted to .realize(). So convert it and rename it to >>>> i82801b11_bridge_realize(). >>>> >>>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> >>>> --- >>>> hw/pci-bridge/i82801b11.c | 9 ++++----- >>>> 1 file changed, 4 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c >>>> index 2404e7e..dca3162 100644 >>>> --- a/hw/pci-bridge/i82801b11.c >>>> +++ b/hw/pci-bridge/i82801b11.c >>>> @@ -44,6 +44,7 @@ >>>> #include "qemu/osdep.h" >>>> #include "hw/pci/pci.h" >>>> #include "hw/i386/ich9.h" >>>> +#include "qapi/error.h" >>>> /*****************************************************************************/ >>>> @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge { >>>> /*< public >*/ >>>> } I82801b11Bridge; >>>> -static int i82801b11_bridge_initfn(PCIDevice *d) >>>> +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp) >>>> { >>>> int rc; >>>> @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d) >>>> goto err_bridge; >>>> } >>>> pci_config_set_prog_interface(d->config, >>>> PCI_CLASS_BRIDGE_PCI_INF_SUB); >>>> - return 0; >>>> + return; >>>> err_bridge: >>>> pci_bridge_exitfn(d); >>> >>> Hi, >>> >>> Since you move to realize, you may want to leverage the errp to add >>> info on errors. >>> >>> Thanks, >>> Marcel >> >> >> Hi, Marcel >> > > Hi! >> Thanks for your quick reply and advice. In fact, I have considered >> adding an error >> message to errp when an error occurs. But I found that >> pci_bridge_ssvid_init() has >> reported a specific info when it fails. If the error is reported >> here again, it seems >> a bit more superfluous, so it's not adopted. > > I agree we don't want to see the error twice, but that means we may want > to pass the error pointer to pci_bridge_ssvid_init and so on. Yes. Callees that report errors differently are a common issue when converting a function to Error. The correct solution is to convert these callees to Error, too. Sometimes, this is just too much to be practical. Then a "shallow" conversion can make sense as a stop-gap, even though it'll result in clearly inferior error reporting. At a glance, this one doesn't look impractical. > One of the main things we achieve when moving to 'realize' is better > error handling, so if we don't do that maybe is not worth it. We need to convert all devices to realize() so we can get rid of the old qdev methods. But you're right in that we better convert them correctly. > You may need to do several changes you didn't intend to > in order to do the error propagation right... Yes. An Error conversion can look simple at first, then balloon into a multi-part series. > > Thanks, > Marcel > >> Of course, output a readable error info >> to make it easier for users is also a good choice. So, are you sure >> you want to do >> that? >> >> Look forward to your feedback and suggestion soon. >> >> Thanks a lot. >> Mao Any help with converting the remaining devices to realize() is much appreciated. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize 2017-05-29 11:37 ` Markus Armbruster @ 2017-05-31 1:46 ` Mao Zhongyi 0 siblings, 0 replies; 6+ messages in thread From: Mao Zhongyi @ 2017-05-31 1:46 UTC (permalink / raw) To: Markus Armbruster, Marcel Apfelbaum; +Cc: qemu-devel, mst Hi, Marcel & Markus On 05/29/2017 07:37 PM, Markus Armbruster wrote: > Marcel Apfelbaum <marcel@redhat.com> writes: > >> On 27/05/2017 9:58, Mao Zhongyi wrote: >>> >>> >>> On 05/26/2017 10:08 PM, Marcel Apfelbaum wrote: >>>> >>>> >>>> On 26/05/2017 15:15, Mao Zhongyi wrote: >>>>> The pci-birdge device i82801b11 still implements the old >>>>> PCIDeviceClass .init() through i82801b11_bridge_init() >>>>> instead of the new .realize(). All devices need to be >>>>> converted to .realize(). So convert it and rename it to >>>>> i82801b11_bridge_realize(). >>>>> >>>>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> >>>>> --- >>>>> hw/pci-bridge/i82801b11.c | 9 ++++----- >>>>> 1 file changed, 4 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c >>>>> index 2404e7e..dca3162 100644 >>>>> --- a/hw/pci-bridge/i82801b11.c >>>>> +++ b/hw/pci-bridge/i82801b11.c >>>>> @@ -44,6 +44,7 @@ >>>>> #include "qemu/osdep.h" >>>>> #include "hw/pci/pci.h" >>>>> #include "hw/i386/ich9.h" >>>>> +#include "qapi/error.h" >>>>> /*****************************************************************************/ >>>>> @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge { >>>>> /*< public >*/ >>>>> } I82801b11Bridge; >>>>> -static int i82801b11_bridge_initfn(PCIDevice *d) >>>>> +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp) >>>>> { >>>>> int rc; >>>>> @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d) >>>>> goto err_bridge; >>>>> } >>>>> pci_config_set_prog_interface(d->config, >>>>> PCI_CLASS_BRIDGE_PCI_INF_SUB); >>>>> - return 0; >>>>> + return; >>>>> err_bridge: >>>>> pci_bridge_exitfn(d); >>>> >>>> Hi, >>>> >>>> Since you move to realize, you may want to leverage the errp to add >>>> info on errors. >>>> >>>> Thanks, >>>> Marcel >>> >>> >>> Hi, Marcel >>> >> >> Hi! >>> Thanks for your quick reply and advice. In fact, I have considered >>> adding an error >>> message to errp when an error occurs. But I found that >>> pci_bridge_ssvid_init() has >>> reported a specific info when it fails. If the error is reported >>> here again, it seems >>> a bit more superfluous, so it's not adopted. >> >> I agree we don't want to see the error twice, but that means we may want >> to pass the error pointer to pci_bridge_ssvid_init and so on. > > Yes. > > Callees that report errors differently are a common issue when > converting a function to Error. The correct solution is to convert > these callees to Error, too. > > Sometimes, this is just too much to be practical. Then a "shallow" > conversion can make sense as a stop-gap, even though it'll result in > clearly inferior error reporting. > > At a glance, this one doesn't look impractical. > >> One of the main things we achieve when moving to 'realize' is better >> error handling, so if we don't do that maybe is not worth it. > > We need to convert all devices to realize() so we can get rid of the old > qdev methods. But you're right in that we better convert them > correctly. > >> You may need to do several changes you didn't intend to >> in order to do the error propagation right... > > Yes. An Error conversion can look simple at first, then balloon into a > multi-part series. > >> >> Thanks, >> Marcel >> >>> Of course, output a readable error info >>> to make it easier for users is also a good choice. So, are you sure >>> you want to do >>> that? >>> >>> Look forward to your feedback and suggestion soon. >>> >>> Thanks a lot. >>> Mao > > Any help with converting the remaining devices to realize() is much > appreciated. > Thanks for your detailed explanation, I think I know what to do. Thanks, Mao ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-31 1:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-26 12:15 [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize Mao Zhongyi 2017-05-26 14:08 ` Marcel Apfelbaum 2017-05-27 6:58 ` Mao Zhongyi 2017-05-27 17:01 ` Marcel Apfelbaum 2017-05-29 11:37 ` Markus Armbruster 2017-05-31 1:46 ` Mao Zhongyi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).