NBD: add nbd_default_export config option #530
Conversation
bignaux
changed the title
|
Check the formatting (clang-format). Running |
|
@rickgaiser : i don't understand because i wrote the .clang-format-ignore for this file previous PR. now it seems it didn't work, i think the easiest way i stop wasting time on that, nbd-protocol.h will be formated OPL side, in a purist way i'd have keep this verbatim from nbdkit, noone care after all. I was in a hurry because i want that code for the week end, since someone could release a memorycard driver this day, and i'd to offer him a way to have user without much hard work on the protocol, that is not yet ready. I think i've more priority on my todolist. EDIT : the scheme i try to break first, is people thinking nbd is for hdd, no and mc driver let people imagine write other drivers. Then nbd is not for device only, and it should really be more known. When i see code for ps2ident, i think, hey guy, divide by 10 or more we have dump via ethernet ... |
|
@bignaux you have issues not in header file but in source file: |
|
Oki, i moved files to prepare integration in ps2sdk, quite all files are now in the lwnbd repo, and i forget to change path in .clang-format-ignore . Fixed. |
| @@ -1401,7 +1404,7 @@ static int loadLwnbdSvr(void) | |||
| if (ret == 0) { | |||
| ret = sysLoadModuleBuffer(&ps2atad_irx, size_ps2atad_irx, 0, NULL); | |||
| if (ret >= 0) { | |||
| ret = sysLoadModuleBuffer(&lwnbdsvr_irx, size_lwnbdsvr_irx, 0, NULL); | |||
| ret = sysLoadModuleBuffer(&lwnbdsvr_irx, size_lwnbdsvr_irx, 4, (char *)&gExportName); | |||
There was a problem hiding this comment.
Is 4 correct here? Shouldn't this be 32 or sizeof(gExportName) ?
EDIT: Shouldn't &gExportName also be gExportName (without the &)
| @@ -191,6 +191,7 @@ unsigned char gDefaultUITextColor[3]; | |||
| hdl_game_info_t *gAutoLaunchGame; | |||
| char gOPLPart[128]; | |||
| char *gHDDPrefix; | |||
| char gExportName[32]; | |||
There was a problem hiding this comment.
This new parameter does not have a default value, and is also never initialized. Shouldn't this be at least initialized to 0?
|
Currently, lwNBD only support old style handshake, so it can't export multiple device and choose it client-side. The only way to use a different device that the hdd0 hardcoded, is to launched it with a default nbd_default_export that is passed to the init() of the server. It could be later change in a callback and have a UI in opl to configure it (but need a callback to get the name of the exports). I did this quickly to allow a contributor to have an easy way to fix his driver, i tested it quickly that was not intented to be documented and used immediatly. Now a big step will be to rewrite the nbd server with event loop based on select(). |
|
Alright, this was your last change for the v1.1 release ;). |
|
hey @rickgaiser i was not forget your review ! but since i don't have good answer right now, you know, i've plan in mind, but people can't guess it. This workaround will be checked, like i'd remove all the printf today according to @AKuHAK previous review. |
| // int ata_device_sce_identify_drive(int device, void *data); | ||
| strcpy(me->super.export_desc, "PlayStation 2 HDD via ATAD"); | ||
| sprintf(me->super.export_name, "%s%d", "hdd", me->device); | ||
| me->super.blocksize = 512; | ||
| me->super.buffer = nbd_buffer; | ||
| me->super.eflags = NBD_FLAG_HAS_FLAGS; | ||
| me->super.eflags = NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_FLUSH; |
There was a problem hiding this comment.
NBD_FLAG_SEND_FLUSH was not functional (no server reply).
NBD: add nbd_default_export config option
NBD: add nbd_default_export config option
Pull Request checklist
Note: these are not necessarily requirements
Pull Request description