Unbind the dependency on each other for tcmu-runner and libtcmu.

[lxbsz]

Preparing to unbind the dependency on each other for tcmu-runner and libtcmu.

1, Move handler register/unregister helpers from tcmu-runner to libtcmu.so to unbind the dependencies.
2, Move tcmur_handler struct from tcmu-runner to libtcmu.so
3, Rename tcmur_handler to tcmulib_backstore_handler.
4, Make libtcmu_priv.h only visible for libtcmu.so
5, Rename some files:
alua.c --> libtcmu_alua.c
alua.h --> libtcmu_alua.h
tcmur_aio.c --> libtcmu_aio.c
tcmur_aio.h --> libtcmu_aio.h
tcmur_device.c --> libtcmu_device.c
tcmur_device.h --> libtcmu_device.h
target.c --> libtcmu_tpg.c
target.h --> libtcmu_tpg.h

6, Delete some files:
tcmu-runner.h
main-syms.txt

7, Add SCSI helpers, and rename the SCSI helpers:
libtcmu_scsi.c
libtcmu_scsi.h
8, Revert "build: drop versionless libtcmu.so symlink"
9, libtcmu: install the headers and revert the old discarding commit: the headers include:
libtcmu.h libtcmu_common.h libtcmu_scsi.h and ccan/ccan/*

[baihuahua]

qemu-tcmu works properly with this patchset but need to define two different handler struct instances: tcmulib_backstore_handler and tcmulib_handler. Could we tune further to export just one handler or merge them?

[lxbsz]

@baihuahua

Here tcmulib_backstore_handler will be exported out to the handlers and the tcmulib_handler will be exported to the daemon like tcmu-runner and qemu-tcmu. And the handlers and the daemon are different parts, so merge them is not a very good idea.

Currently the qemu-tcmu have integrated the qemu handler and daemon process together, maybe we should separate the code as different components.

[lxbsz]

@mikechristie @pkalever

Please help review, this is the first version of adding the libtcmu library back.

[mikechristie]

Could you post a link to the current qemu-tcmu code?

[lxbsz]

Could you post a link to the current qemu-tcmu code?

Yeah, please see: https://github.com/lxbsz/qemu/commits/qemu-tcmu, this code comes from @baihuahua.

Thanks,

[baihuahua]

Hi Mike, Does this change still make sense? What should we do to make libtcmu usable by both tcmu-runner and qemu-tcmu? @mikechristie

[mikechristie]

Sorry. I am behind. I am trying to get to this this week.

[mikechristie]

Here is some general questions/comments:

• I am not so sure I understand the end goal for the scsi processing. Is qemu-tcmu going to use the qemu or libtcmu scso helpers? For example for the final qemu_tcmu_handle_cmd are you planning on use the libtcmu scsi helpers or some qemu ones?

Are you trying to use the libtcmu scsi helper and so that is why the patches merged the tcmu-runner and libtcmu device/handler structs?

For some reason I thought you guys were going to do the opposite.

• With this new design do you now have to do a tcmulib_handler and also a tcmulib_backstore_handler? We are dropping support for the daemon type where it only did a tcmulib_handler (consumer.c/tcmu-synthesizer.c)?

• If we do go this route instead of tcmulib_backstore_handler, I think we can just shorten this to tcmulib_backstore.

[baihuahua]

Actually the plan of qemu-tcmu is to use SCSI code in QEMU in the end, but this QEMU SCSI code needs to tune for qemu-tcmu to use so for now qemu-tcum still uses SCSI helpers from litcmu/tcmu-runner. But if it's too hard to seperate tcmu-runner and libtcmu because of these SCSI helpers, qemu-tcmu can change to use SCSI code in QEMU or reimplement several necessary SCSI commands right now, so libtcmu shall just include DBUS registeration and TCMU protocol handling code, without SCSI helper code. By doing this libtcmu will be sharable between tcmu-runner and qemu-tcmu and tcmu-runner doesn't need to change too much. I think this should be more reasonable and more easier to achieve. What do you think Mike?

[lxbsz]

@mikechristie
Sorry for late, these days I was busying for other workloads.

Here is some general questions/comments:

I am not so sure I understand the end goal for the scsi processing. Is qemu-tcmu going to use the qemu or libtcmu scso helpers? For example for the final qemu_tcmu_handle_cmd are you planning on use the libtcmu scsi helpers or some qemu ones?
Are you trying to use the libtcmu scsi helper and so that is why the patches merged the tcmu-runner and libtcmu device/handler structs?

Currently, qemu-tcmu is only using some of the sync scsi helpers which are also called handle_sync_cmd(). And qemu-tcmu must use qemu's APIs to implement other scsi helpers to support some feature like the clone. And finally it will add it's own basic helpers of all the scsi commands and then switch to them. This should be the goal of qemu-tcmu.

These days I was thinking to separate the sysfs/uio and the dbus helpers from the tcmu-runner into libtcmu.so. And finally the libtcmu.so will mainly include the following 3 parts:
[1] sysfs/uio hanlding helpers
[2] dbus helpers
[3] the sync scsi helpers

For some reason I thought you guys were going to do the opposite.

With this new design do you now have to do a tcmulib_handler and also a tcmulib_backstore_handler?
From the current qemu-tcmu code, it must, but we can change the qemu-tcmu design.

We are dropping support for the daemon type where it only did a tcmulib_handler (consumer.c/tcmu-synthesizer.c)?

For now the qemu-tcmu is also using daemon type too.

If we do go this route instead of tcmulib_backstore_handler, I think we can just shorten this to tcmulib_backstore.
Thanks.

[baihuahua]

And finally it will add it's own basic helpers of all the scsi commands and then switch to them. This should be the goal of qemu-tcmu.

To be precise, much of these SCSI command helpers have implemented in QEMU as there's a requirement of full-functional SCSI disk emulation for QEMU.

With this new design do you now have to do a tcmulib_handler and also a tcmulib_backstore_handler?

From the current qemu-tcmu code, it must, but we can change the qemu-tcmu design.

Emmm..., yes, qemu-tcmu is still in RFC and we can change it as we need. But i'm afraid such changing qemu-tcmu to apply the model of tcmu-runner is not the right direction. Because the motivation of tcmu-runner and qemu-tcmu are different. One important feature/goal of tcmu-runner is to make it easy to develop various handlers like rbd,gluster so tcmu-runner has tcmulib_handler for a daemon and tcmulib_backstore_handler for the handlers. While qemu-tcmu doesn't have such a requirement as QEMU has already supported them so it's a bit weird for qemu-tcmu to have such two handlers.

[mikechristie]

Hey @lxbsz @baihuahua

I am not sure if you guys were waiting for me to comment on this or not. I was actually waiting for you guys to finish discussing it :)

If you guys are waiting on me, my opinion would be:

1. For the initial lib do as little as possible. As long as we design it right it should be easier to expand the lib API instead of having to support bad API calls/structs that we made mistakes on.

It seems we all agree we need the lib to handle the very basics: uio, configfs, and netlink.

So we can just take the libtcmu code that did this stuff and make it a lib.

2. It seems that you guys do not agree on the SCSI parts. I would like to share SCSI code as much as possible, but for the initial release I think we can skip it. Just have qemu-tcmu plan to use the native qemu SCSI code and have tcmu-runner use its code.

So can we skip adding the SCSI api.c parts into the lib initially? You guys can then debate this later.

3. Just do the most basic daemon support initially. No merging of tcmu-runner handlers and libtcmu handlers. So just support what is needed for gluster: fix tcmu_get_attribute error check #1. That is basically what we had in the original lib.

We can at least then start with a stable lib that supports enough to create, delete and config devices, and handle IO.

[lxbsz]

@mikechristie
Actually these days was busy for the recent release stuff. Not very much time on this.

1. For the initial lib do as little as possible. As long as we design it right it should be easier to expand the lib API instead of having to support bad API calls/structs that we made mistakes on.
   It seems we all agree we need the lib to handle the very basics: uio, configfs, and netlink.

Yeah, this looks fine to me.

2. It seems that you guys do not agree on the SCSI parts. I would like to share SCSI code as much as possible, but for the initial release I think we can skip it. Just have qemu-tcmu plan to use the native qemu SCSI code and have tcmu-runner use its code.
   So can we skip adding the SCSI api.c parts into the lib initially? You guys can then debate this later.

Yeah, currently the qemu-tcmu rfc code is only using very small part of the SCSI helpers. To simplify it, I agree to skip it firstly, and then debate this later. And we'd better reuse the SCSI part as possible as we can or we must recode tons of relevant same features that we have already running and test for a long time.

3. Just do the most basic daemon support initially. No merging of tcmu-runner handlers and libtcmu handlers. So just support what is needed for gluster: fix tcmu_get_attribute error check #1. That is basically what we had in the original lib.
   We can at least then start with a stable lib that supports enough to create, delete and config devices, and handle IO.

Will split the PRs into small ones.

Thanks very much Mike.

[mikechristie]

Hey,

Attached could be the basis for the initial base lib functions we will support:

0001-libtcmu3-header-example.txt

Note this patch requires this patch

0618ade

which drops some dbus stuff.

So basically we move the functions listed in the headers in 0001-libtcmu3-header-example.txt and put them in new files:

libtcmu3 files:
libtcmu3.c - core functions that do not belong anywhere specific. Functions like tcmulib_get_next_command, tcmulib_initialize, etc, and misc functions they call like open_devices. We may want to put the netlink functions into a new file netlink.c just to make the organization nicer.
configfs.c - configfs helpers. We can just take this entire file.

Notes.

1. One issue we need to eventually decide is the return code used in tcmulib_command_complete. In the example header it is a tcmu_cmd_status, but that is going to be a pain to have the libtcmu3 users have to send a patch to update libtmcu3 every time they want to add a new error string. We probably want them to just set the sense and return a SCSI status code like before.

So we should move the tcmu_cmd_status stuff to tcmu-runner specific code.

2. This patch/plan is just the most basic info/layout/plan. I am open to other ideas.

[mikechristie]

I guess a funny/dumb thing on my part is my proposal above is just basically bringing back the old libtcmu minus the SCSI helpers. libtcmu3.c would be almost exactly the same as libtcmu.c and libtcmu_common3.h and libtcmu3.h in my patch are really similar to libtcmu_common.h/libtcmu.h

So we probably do not need to rename them. We could just move the scsi and iovec stuff from libtcmu_common.h and libtcmu.h and related .c files to new files that only tcmu-runner uses and then just bump the lib version:

set_target_properties(tcmu
PROPERTIES
SOVERSION "3"
)

I can send a patch for this.

[baihuahua]

I can send a patch for this.

That would be great.

With commit 0618ad every daemon like tcmu-synthesizer and qemu-tcmu need to acquire the DBUS name "org.kernel.TCMUService1" on their own, right?

[lxbsz]

set_target_properties(tcmu
PROPERTIES
SOVERSION "3"
)

I can send a patch for this.

Yeah, that looks good to me and please go ahead Mike.
I will begin to start this since next week.

Thanks.

[mikechristie]

I made a mistake linking that patch:

0618ade

to the libtcmu issue here and you can ignore it.

They are not really related right now for the initial release. The dbus code being removed in that patch is just not used by anyone anymore. It was used by dbus users like targetcli to have the daemon load a handler dynamically. I think the problem was that it was added but we never fully maintained it due the only user not using it anymore.

I think going forward it is a useful to have something like it in the lib, and I think we should add dbus helpers to the lib eventually. For the initial release though I think we can hold off unless you have a immediate need.

[lxbsz]

Yeah, Agree with that.

For the initial version let's make it as simple as possible，then push this forward.

@mikechristie Will you send the initial version of the libtcmu3 these days ?

[baihuahua]

Sorry i'm behind. If we decide to not include the dbus code into the initial release of new libtcmu, 0618ade should not be merged because qemu-tcmu need the code in that patch to register onto DBUS to cooperate with targetcli as a backstore. Tcmu-synthesizer should also be in this same situation. If we include dbus code into the initial release, that's okey to remove code in that patch as daemons could grab the dbus on their own.

[mikechristie]

Sorry i'm behind. If we decide to not include the dbus code into the initial release of new libtcmu, 0618ade should not be merged because qemu-tcmu need the code in that patch to register onto DBUS to cooperate with targetcli as a backstore

targetcli only uses dbus for check_config and to get a list of handlers.

1. tcmu-runner does not use check_config anymore so it always returns success, so that does not matter.

2. targetcli does need to be able to get a list of handlers. That works with or without that patch.

That patch is not related to the discussion. It was an accident on my fault for referencing it. The patch just removes the ability to for something like targetcli (targetcli does not actually use it) to tell tcmu-runner to load a handler after the initial startup (when tcmu-runner starts it loads all the handlers in the lib/tcmu-runner dir). The problem is that it looks like it was implemented early on then never updated except for some security issues. It does not actually work like how you would want where you can load/unload handlers for something like a dynamic handler update or operation like that which we do need at some point in the future.

[baihuahua]

OK, let's drop that patch out of the picture. What's your plan of this new initial libtcmu, Mike? That would be very great if we can see it soon.

[mikechristie]

We are just finishing up QEing a release at work, so I hope to have it done by the end of the month.

Let me know if you want to work on it. I thought you mentioned it above, but was not sure. If you want to make it it is ok with me.

[baihuahua]

OK, i can make it. Will send PR ASAP.