Hi Corey,

Thank you for reviewing the patch!
I'll revise it and put it inline and send you a new patch later.

Kind regards,
Yunpeng Yang


________________________________
From: Corey Minyard <[email protected]>
Sent: Wednesday, October 15, 2025 00:17
To: Yunpeng Yang <[email protected]>
Cc: [email protected] <[email protected]>; [email protected] 
<[email protected]>; Mark Cave-Ayland <[email protected]>; 
Jonathan Davies <[email protected]>
Subject: Re: Add support for retrieving fake LAN config from `ipmi-bmc-sim` 
device

!-------------------------------------------------------------------|
  CAUTION: External Email

|-------------------------------------------------------------------!

On Tue, Oct 14, 2025 at 12:19:53PM +0000, Yunpeng Yang wrote:
> Hello Corey,
>
> I hope you are all well.
>
> I've finally come up with a patch for implementing the fake LAN 
> configurations for the device ipmi-bmc-sim. My apologies for the long long 
> delay. And thank you for your patience.
>
> Please find attached the patch file. The patch is base on commit: bd6aa0d1e5 
> ("Merge tag 'staging-pull-request' of 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_peterx_qemu&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=gyJEZb_gH0rRUcKZUkVe15Lqy_BoDVLs3C-LhiDdmek&m=rfmLapMDI4Ee_0kJdoHlr3SDVhA8ZRzQ8uGSoMPqV7u-WryqTxU7x0I2YHUjPqCF&s=qrpat3zQSFjeayoVw4uZAnIyhoVgtck2A1C-AG9CJO0&e=
>   into staging"). Could you please review the patch and let me know your 
> opinions?

The patch is mostly ok, the general principles are sound.

A few things:

Can you put the patch inline in the email instead of attaching it?  In
general, it is hard to deal with attachments when commenting on things.
Use git-send-email or git-format-patch or something like that.  You can
look in the Linux kernel docs for how to do this.

I could not find a check on the lan.channel value to make sure it is in
range and doesn't conflict with an existing channel.  If a check is not
there it needs to be done.

I would like to keep the behavior as close as possible to the current
behavior.  If a channel is not defined, get_lan_config() and
set_lan_config() should return the same value as if the command was not
defined.  I think get_channel_access() is ok as it is.

You need to add documentation in qemu-options.hx.

Can you copy Cornelia Huck <[email protected]> on the next
version?  The IBM people are the biggest users of the ipmi code,
and I don't want this to catch them off guard.

If the patch gets too big, feel free to split it into multiple parts.

Thanks,

-corey

>
> Looking forward to hearing back from you.
>
> Kind regards,
> Yunpeng Yang
>
>
>
> ________________________________
> From: Corey Minyard <[email protected]>
> Sent: Sunday, August 03, 2025 14:22
> To: Yunpeng Yang <[email protected]>
> Cc: [email protected] <[email protected]>; [email protected] 
> <[email protected]>; Mark Cave-Ayland <[email protected]>; 
> Jonathan Davies <[email protected]>
> Subject: Re: Add support for retrieving fake LAN config from `ipmi-bmc-sim` 
> device
>
> On Sun, Aug 3, 2025 at 6: 16 AM Yunpeng Yang <yunpeng. yang@ nutanix. com> 
> wrote: Hello Corey Minyard, I hope you are all well. Could I ask for your 
> opinion on whether it is worthing implementing a fake LAN config for device 
> ipmi-bmc-sim ?
> ZjQcmQRYFpfptBannerStart
> CAUTION: External Email
>
> ZjQcmQRYFpfptBannerEnd
> On Sun, Aug 3, 2025 at 6:16 AM Yunpeng Yang 
> <[email protected]<mailto:[email protected]>> wrote:
> Hello Corey Minyard,
>
> I hope you are all well.
>
> Could I ask for your opinion on whether it is worthing implementing a fake 
> LAN config for device ipmi-bmc-sim ? (Details are in my last email, which is 
> also included below).
>
> During my work over last month, I found that QEMU already has ipmi-bmc-extern 
> which supports comprehensive BMC simulation, including LAN config faking. But 
> ipmi-bmc-sim is more light-weight and easier to set up. So I think it still 
> has some value in implementing LAN config for ipmi-bmc-sim . Could you please 
> share your views?
>
>
> Well, there is no LAN interface, so I didn't see any need to add that.  The 
> values would not be permanent.  But it would be harmless to add, so I'd be ok 
> with a patch to do this.
>
> The external interface with ipmisim from the OpenIPMI library provides a 
> pretty comprehensive solution.
>
> -corey
>
> Best regards,
> Yunpeng Yang
>
>
> ________________________________
> From: Yunpeng Yang <[email protected]<mailto:[email protected]>>
> Sent: Friday, June 27, 2025 18:13
> To: [email protected]<mailto:[email protected]> 
> <[email protected]<mailto:[email protected]>>
> Cc: [email protected]<mailto:[email protected]> 
> <[email protected]<mailto:[email protected]>>; Mark Cave-Ayland 
> <[email protected]<mailto:[email protected]>>; Jonathan 
> Davies <[email protected]<mailto:[email protected]>>
> Subject: Add support for retrieving fake LAN config from `ipmi-bmc-sim` device
>
> Hello Corey Minyard,
>
> I hope this email finds you well.
>
> I'm currently adding LAN-configs-retrieval support to the QEMU ipmi-bmc-sim 
> device. And I hope to merge the modifications upstream after it's finished. 
> Could you please check the attached patch file of the draft code and share 
> your opinions and advice?
>
> In my work, we need to run tools like "ipmitool lan print" on a VM for 
> testing purposes. However, QEMU internal BMC simulator device 
> (`ipmi-bmc-sim`) does not support retrieving LAN configs from it. I have to 
> implement two IPMI commands so that the device can now work with ipmitool. 
> The LAN config values are faked, but for testing purposes this is not a 
> problem. I believe other people may also have the same need, so it's worth 
> getting merged upstream.
>
> The fake BMC LAN config values are currently hard coded into the code. My 
> plan is to add a parameter to the device, which is a file containing user 
> designated values. The device then reads the file and returns those values as 
> LAN configs. This is similar to sdrfile for sensor data and frudatafile for 
> FRU data.
>
> Looking forward to hearing your thoughts.
> Have a nice weekend.
>
> Kind regards,
> Yunpeng Yang


Reply via email to