Dead virtual media code in bmcweb

[edtanous]

The nbd proxy implementation present within bmcweb never had it's backend upstreamed. As such, this is dead code to us, and could be removed.

Code in question is here:
https://github.com/openbmc/bmcweb/blob/master/include/nbd_proxy.hpp

And I suspect could be completely removed with no impact to the openbmc project.

[gtmills]

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/40766 disables this code

[Kitsok]

Just to confirm that this code is used by us (Yandex).

[gtmills]

Just to confirm that this code is used by us (Yandex).

Can you help upstream the backend?
Because without an upstreamed backend we will eventually remove from bmcweb.

[Kitsok]

Just to confirm that this code is used by us (Yandex).

Can you help upstream the backend?
Because without an upstreamed backend we will eventually remove from bmcweb.

It's Intel code (while in public repository), I've asked them to upstream.

[edtanous]

Just to confirm that this code is used by us (Yandex).

Can you help upstream the backend?
Because without an upstreamed backend we will eventually remove from bmcweb.

It's Intel code (while in public repository), I've asked them to upstream.

Please ask them to start by writing a design doc; There's already a virtual media implementation, so we need to understand why this one is better, and needs to diverge from the already upstreamed one. I'm sure there are good reasons, but we need to get those written down.

[Kitsok]

There's already a virtual media implementation

I think Intel implementation fits well to what is described here as legacy mode: https://github.com/openbmc/docs/blob/master/designs/virtual-media.md
I couldn't find other implementation in OpenBMC repository, could you please point its' location?

[edtanous]

bmcweb/meson_options.txt

Line 5 in 90e97e1

option('vm-websocket', type : 'feature', value : 'enabled', description : '''Enable the Virtual Media WebSocket. Path is \'/vm/0/0\'to open the websocket. See https://github.com/openbmc/jsnbd/blob/master/README.''')

[edtanous]

Several months after disabling this, and no progress on upstreaming the backend or a design doc. If no one is interested in this code, it would help the complexity to delete it, which I'm currently intending on doing. At such time as a backend is upstreamed, we can open a new patchset to re-add the feature.

[edtanous]

We're now several years into disabling this, and some progress has been made on getting the backend upstreamed, but not much.

Recent changes have gone in that make me far less worried about the impact of changes in that module, and to improve the reliability, but it still has no backend.

[NguyenTanNhutQuang]

@edtanous and @Kitsok,
I try to understand the whole picture here.
Virtural media has two modes, legacy and proxy mode. There are several components for each mode to enable them to work.
Legacy mode components:

• bmcweb
• virtual media service (this is intel's discontinued repository)
• NBDkit (NBD server)
• NBD (NBD client)
• uDev
• USB Gadget
  Proxy mode components:
• bmcweb
• virtual media service (this is intel's discontinued repository)
• jsnbd (NBD server, and NBD client)
• uDev
• USB Gadget
  So, what is the backend of proxy mode as you mentioned? Please correct me if I am missing anything. Thanks.

[Kitsok]

So, what is the backend of proxy mode as you mentioned?

Not sure if this answers your question, but...
We use two modes of operation of VM:

• USB gadget is fed from the client (then it looks like image->clients' browser->JS->bmcweb->.....)
• USB gadget is fed from an external server (image->https server->curl->jsnbd->... or image->https server->curl->nbdkit->....)

[edtanous]

I wasn't a part of the discussion on virtual media when the bmcweb backend was merged. You should likely look to the people that pushed and/or approved it.

[NguyenTanNhutQuang]

I found that these two commits were added to enable BMCWEB_ENABLE_VM_NBDPROXY
107077d
c0a1c8a

But the build flag BMCWEB_ENABLE_VM_NBDPROXY is a bit missleading since the code of these commits are for implementing Redfish schema of virtual media or the front end of the Redfish virtual media. The backend of these code are implemented by Intel at this repo https://github.com/Intel-BMC/virtual-media which is discontinued.

Now, if we remove the flag BMCWEB_ENABLE_VM_NBDPROXY and its related code (the two commits mentioned above), the feature virtual media via redfish will not work anymore.

I wonder if nobody is using Redfish virtual media?

[Kitsok]

- mention@ Hello! We use it in production, thus if the code is removed, I'll have to include it back using local patches. Thank you! 18.09.2023, 11:44, "NguyenTanNhutQuang" ***@***.***>: I found that these two commits were added to enable BMCWEB_ENABLE_VM_NBDPROXY But the build flag BMCWEB_ENABLE_VM_NBDPROXY is a bit missleading since the code of these commits are for implementing Redfish schema of virtual media or the front end of the Redfish virtual media. The backend of these code are implemented by Intel at this repo https://github.com/Intel-BMC/virtual-media which is discontinued.Now, if we remove the flag BMCWEB_ENABLE_VM_NBDPROXY and its related code (the two commits mentioned above), the feature virtual media via redfish will not work anymore.I wonder if nobody is using Redfish virtual media?—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>  -- Best regards,Konstantin Klubnichkin,lead firmware engineer,server hardware R&D group,Yandex Europe B.V.tel: +31-684-515-740 

[edtanous]

So, what is the backend of proxy mode as you mentioned?

I missed this question the first time around. The virtual media service that you mention in your list. The other things in the list are already well-maintained open source projects. As you note, Intels project is completely discontinued and unmaintained, and given the lack of interest in getting it upstreamed and maintained tells me that we can remove this code from bmcweb.

We use it in production
.... I'll have to include it back using local patches.

If that's the case, it would help a lot if you could please volunteer in getting it reviewed, tested, coded in line with the coding standard, and getting a maintainer that's interested in it added to the reviewers. As-is, it is dead, untestable and unmaintainable code that I'm not sure if works. Patches like this https://gerrit.openbmc.org/c/openbmc/bmcweb/+/66298 get very few reviewers, and no test results, which doesn't seem sustainable. With that said, apparently it works in your environment, so clearly it's of some use.

the feature virtual media via redfish will not work anymore

The feature never worked in an upstream build. As you say, it only worked in the Intel-BMC fork.

[NguyenTanNhutQuang]

The nbd proxy implementation present within bmcweb never had it's backend upstreamed. As such, this is dead code to us, and could be removed.

Code in question is here: https://github.com/openbmc/bmcweb/blob/master/include/nbd_proxy.hpp

And I suspect could be completely removed with no impact to the openbmc project.

@edtanous I misunderstand your first comment, you mean that the code in nbd_proxy.hpp will be removed only, is it?

[thangqn-ampere]

I wonder if we really need to have backend application. As I understand, the InsertMedia action has information needed for the ISO file from SMB or NFS. Then, we just need to mount the ISO file to the RootFS (not sure about current mount point now) and then execute the /etc/nbd-proxy/state script (stored at meta-phosphor/recipes-connectivity/jsnbd/jsnbd/state_hook) with start option to create virtual USB device, similar to the WebUI virtual-media.

[thangqn-ampere]

I wonder if we really need to have backend application. As I understand, the InsertMedia action has information needed for the ISO file from SMB or NFS. Then, we just need to mount the ISO file to the RootFS (not sure about current mount point now) and then execute the /etc/nbd-proxy/state script (stored at meta-phosphor/recipes-connectivity/jsnbd/jsnbd/state_hook) with start option to create virtual USB device, similar to the WebUI virtual-media.

Let ignore my comment. This issue discusses about proxy mode and my comment is for legacy.

[NguyenTanNhutQuang]

The nbd proxy implementation present within bmcweb never had it's backend upstreamed. As such, this is dead code to us, and could be removed.
Code in question is here: https://github.com/openbmc/bmcweb/blob/master/include/nbd_proxy.hpp
And I suspect could be completely removed with no impact to the openbmc project.

@edtanous I misunderstand your first comment, you mean that the code in nbd_proxy.hpp will be removed only, is it?

Hi @edtanous,
Good day! Just a soft remind, could you please confirm my question above? Thank you.