Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscellaneous Unixext minor updates #6132

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Miscellaneous Unixext minor updates #6132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5c76096
Fix indentation and spaces
freddy77 Nov 22, 2024
c254c5c
Use caml_stat_strdup instead of caml_stat_alloc+strcpy
freddy77 Nov 22, 2024
5e3f5cc
Call caml_stat_free inside blocking section
freddy77 Nov 22, 2024
d7ebcdb
Add a test for Unixext.Direct module
freddy77 Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/unixext_test.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,26 @@ let test_select =
in
true

let test_direct =
let gen = Gen.unit in
Test.make ~count:1 ~name:__FUNCTION__ gen @@ fun _ ->
let fn = "test_file_direct" in
let buf = Bytes.make 10000 'x' in
Unixext.Direct.with_openfile fn [Unix.O_RDWR; Unix.O_CREAT] 0o600 (fun f ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan on using this module in a future PR?

There is a lot of dead code in XAPI unfortunately, and in this case the entire Unixext.Direct module appears to be dead code. If I drop the entire module from the interface and impl, then XAPI still builds.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimizations and this testcase looks correct, but if this is dead code we could also simplify things and delete the unused C stubs, and modules.

let res = Unixext.Direct.write f buf 0 4096 in
QCheck2.assume (res = 4096)
) ;
Unix.unlink fn ;
true

let tests =
[test_select; test_proxy; test_time_limited_write; test_time_limited_read]
[
test_select
; test_proxy
; test_time_limited_write
; test_time_limited_read
; test_direct
]

let () =
(* avoid SIGPIPE *)
Expand Down
7 changes: 3 additions & 4 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,20 @@ CAMLprim value stub_stdext_unix_open_direct(value path, value flags, value perm)
char * p;

cv_flags = caml_convert_flag_list(flags, open_flag_table);

#ifdef O_DIRECT
cv_flags |= O_DIRECT;
#endif
p = caml_stat_alloc(caml_string_length(path) + 1);
strcpy(p, String_val(path));
p = caml_stat_strdup(String_val(path));
/* open on a named FIFO can block (PR#1533) */
caml_enter_blocking_section();
fd = open(p, cv_flags, Int_val(perm));
#ifndef O_DIRECT
if (fd != -1)
ret = fcntl(fd, F_NOCACHE);
#endif
caml_leave_blocking_section();
caml_stat_free(p);
caml_leave_blocking_section();
if (fd == -1) uerror("open", path);
#ifndef O_DIRECT
if (ret == -1) uerror("fcntl", path);
Expand Down
98 changes: 49 additions & 49 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include <sys/ioctl.h>
#include <sys/statvfs.h>
#if defined(__linux__)
# include <linux/fs.h>
# include <linux/fs.h>
#endif

#include <caml/mlvalues.h>
Expand All @@ -41,28 +41,28 @@
/* Set the TCP_NODELAY flag on a Unix.file_descr */
CAMLprim value stub_unixext_set_tcp_nodelay (value fd, value bool)
{
CAMLparam2 (fd, bool);
int c_fd = Int_val(fd);
int opt = (Bool_val(bool)) ? 1 : 0;
if (setsockopt(c_fd, IPPROTO_TCP, TCP_NODELAY, (void *)&opt, sizeof(opt)) != 0){
uerror("setsockopt", Nothing);
}
CAMLreturn(Val_unit);
CAMLparam2 (fd, bool);
int c_fd = Int_val(fd);
int opt = (Bool_val(bool)) ? 1 : 0;
if (setsockopt(c_fd, IPPROTO_TCP, TCP_NODELAY, (void *)&opt, sizeof(opt)) != 0){
uerror("setsockopt", Nothing);
}
CAMLreturn(Val_unit);
}

CAMLprim value stub_unixext_fsync (value fd)
{
CAMLparam1(fd);
int c_fd = Int_val(fd);
int rc;

caml_release_runtime_system();
rc = fsync(c_fd);
caml_acquire_runtime_system();
if (rc != 0) uerror("fsync", Nothing);
CAMLreturn(Val_unit);
CAMLparam1(fd);
int c_fd = Int_val(fd);
int rc;

caml_release_runtime_system();
rc = fsync(c_fd);
caml_acquire_runtime_system();
if (rc != 0) uerror("fsync", Nothing);
CAMLreturn(Val_unit);
}


CAMLprim value stub_unixext_blkgetsize64(value fd)
{
Expand All @@ -84,10 +84,10 @@ CAMLprim value stub_unixext_blkgetsize64(value fd)

CAMLprim value stub_unixext_get_max_fd (value unit)
{
CAMLparam1 (unit);
long maxfd;
maxfd = sysconf(_SC_OPEN_MAX);
CAMLreturn(Val_int(maxfd));
CAMLparam1 (unit);
long maxfd;
maxfd = sysconf(_SC_OPEN_MAX);
CAMLreturn(Val_int(maxfd));
}

#if defined(__linux__)
Expand All @@ -102,41 +102,41 @@ CAMLprim value stub_unixext_set_sock_keepalives(value fd, value count, value idl
{
CAMLparam4(fd, count, idle, interval);

freddy77 marked this conversation as resolved.
Show resolved Hide resolved
int c_fd = Int_val(fd);
int optval;
socklen_t optlen=sizeof(optval);
optval = Int_val(count);
if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPCNT, &optval, optlen) < 0) {
uerror("setsockopt(TCP_KEEPCNT)", Nothing);
}
#if defined(__linux__)
optval = Int_val(idle);
if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPIDLE, &optval, optlen) < 0) {
uerror("setsockopt(TCP_KEEPIDLE)", Nothing);
}
int c_fd = Int_val(fd);
int optval;
socklen_t optlen=sizeof(optval);

optval = Int_val(count);
if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPCNT, &optval, optlen) < 0) {
uerror("setsockopt(TCP_KEEPCNT)", Nothing);
}
#if defined(__linux__)
optval = Int_val(idle);
if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPIDLE, &optval, optlen) < 0) {
uerror("setsockopt(TCP_KEEPIDLE)", Nothing);
}
#endif
optval = Int_val(interval);
if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPINTVL, &optval, optlen) < 0) {
uerror("setsockopt(TCP_KEEPINTVL)", Nothing);
}
optval = Int_val(interval);
if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPINTVL, &optval, optlen) < 0) {
uerror("setsockopt(TCP_KEEPINTVL)", Nothing);
}

CAMLreturn(Val_unit);
CAMLreturn(Val_unit);
}

void unixext_error(int code)
{
static const value *exn = NULL;

if (!exn) {
exn = caml_named_value("unixext.unix_error");
if (!exn)
caml_invalid_argument("unixext.unix_error not initialiazed");
}
caml_raise_with_arg(*exn, Val_int(code));
static const value *exn = NULL;

if (!exn) {
exn = caml_named_value("unixext.unix_error");
if (!exn)
caml_invalid_argument("unixext.unix_error not initialiazed");
}
caml_raise_with_arg(*exn, Val_int(code));
}

CAMLprim value stub_statvfs(value filename)
CAMLprim value stub_statvfs(value filename)
{
CAMLparam1(filename);
CAMLlocal1(v);
Expand Down
8 changes: 4 additions & 4 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_write_stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ CAMLprim value stub_stdext_unix_write(value fd, value buf, value vofs, value vle
written = 0;
while (len > 0) {
numbytes = len > UNIX_BUFFER_SIZE ? UNIX_BUFFER_SIZE : len;
ret = posix_memalign(&iobuf, PAGE_SIZE, numbytes);
if (ret != 0)
uerror("write/posix_memalign", Nothing);
ret = posix_memalign(&iobuf, PAGE_SIZE, numbytes);
if (ret != 0)
uerror("write/posix_memalign", Nothing);

memmove (iobuf, &Byte(buf, ofs), numbytes);
caml_enter_blocking_section();
ret = write(Int_val(fd), iobuf, numbytes);
caml_leave_blocking_section();
free(iobuf);
free(iobuf);

if (ret == -1) {
if ((errno == EAGAIN || errno == EWOULDBLOCK) && written > 0) break;
Expand Down
Loading